Mark, On 2/19/15 1:41 PM, Mark Thomas wrote: > On 19/02/2015 17:45, schu...@apache.org wrote: >> Author: schultz >> Date: Thu Feb 19 17:45:34 2015 >> New Revision: 1660953 >> >> URL: http://svn.apache.org/r1660953 >> Log: >> Back-port r1660924 to fix >> https://bz.apache.org/bugzilla/show_bug.cgi?id=57540 >> Expose TLS protocol via a request attribute. > > Sorry for the delayed feedback. > > <snip/> > >> @@ -882,7 +883,7 @@ public class Request >> if(attr != null) { >> return attr; >> } >> - if( isSSLAttribute(name) ) { >> + if( isSSLAttribute(name) || >> name.equals(SSLSupport.PROTOCOL_VERSION_KEY)) { > > This should be part of the isSSLAttribute() test. I'd it to Globals to > for consistency with the other attributes.
I avoided adding it to isSSLAttribute because of this Javadoc for that
method:
/**
* Test if a given name is one of the special Servlet-spec SSL
attributes.
*/
Since this isn't in the spec, I kept is separate. I think it's a
reasonable thing to add to the spec: any chance you'd be willing to
bring it to their attention?
> Hmm. That does mean we end up with multiple definitions. That seems wrong.
>
> I haven't checked how reasonable the following is...
> How about for trunk drop the Globals constants and use the ones from
> SSLSupport. For the back-port, deprecate the the Globals ones and define
> them in terms of SSLSupport.
SSLSupport has varying levels of, er, support, from Tomcat 7 onward.
Your work with refactoring everything in trunk was great.. back-porting
required duplications of lots of that stuff -- feel free to look at the
diffs I committed.
I'll wait for it to roll-around in your head a bit more; I trust your
architectural view and I'm not sure my opinion would be well-informed.
Once you decide, I can make any changes required.
Thanks,
-chris
signature.asc
Description: OpenPGP digital signature
