On 19/02/2015 19:31, Christopher Schultz wrote: > 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?
If you raise the Jira I can comment positively on it. Better for two people to request it than one. My commendation is to change the comment to Servlet spec + Tomcat specific SSL attributes. >> 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. We definitely should only be defining constants once as far as possible. On that basis my suggestion above stands but if you can see a better way go for it. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
