https://issues.apache.org/bugzilla/show_bug.cgi?id=54729
Bug ID: 54729
Summary: new HttpParser.parseAuthorizationBasic method
Product: Tomcat 8
Version: trunk
Hardware: PC
OS: Linux
Status: NEW
Severity: enhancement
Priority: P2
Component: Catalina
Assignee: dev@tomcat.apache.org
Reporter: br...@pingtoo.com
Classification: Unclassified
Created attachment 30083
--> https://issues.apache.org/bugzilla/attachment.cgi?id=30083&action=edit
svn diff: update to parser and new test class
While working on https://issues.apache.org/bugzilla/show_bug.cgi?id=54190, I
left several TODO's where the current BasicAuthenticator did not handle some
test cases properly. Mark suggested I use the
HttpParser.parseAuthorizationDigest method as a model for re-implementing Basic
authentication.
I am submitting a preliminary change which will permit moving the parsing logic
out of BasicAuthenticator and into HttpParser. I have also written a new test
class to exercise all relevant test cases in the new parser. I have implemented
these changes in a manner that is 100% backward compatible, because I would
like to get them committed before I submit my dependent changes to
BasicAuthenticator and its own test class.
When reviewing the attached patches, please take the following into
consideration:
1. Although I started my implementation by trying to write efficient code, I
eventually rejected that approach. The change I am now submitting sacrifices
efficiency in favour of clarity. Its logic is analogous to that of the
parseAuthorizationDigest method, even though the processing requirents are
somewhat different. This has the advantage that the two
Authenticator.authenticate methods will eventually have quite similar logic.
2. My new tests expose several deficiencies and bugs in
org.apache.catalina.util.Base64. I might be mistaken, but I haven't been able
to find a test class for Base64. Rather than fix the bugs and possibly trigger
side effects in other users of the class, I have decided to implement some
temporary defensive logic in the new parser. This is a bit ugly, but is not
intended to be present for very long.
3. I would like to tackle the Base64 class later, by writing a full set of unit
tests. I anticipate refactoring the two decode methods to eliminate duplicate
logic, as well as providing a more efficient signature for use by
parseAuthorizationBasic. I will refer to the latest commons Base64 class and
its test cases. I will fix the current bugs and then be in a position to remove
the defensive code in the parser.
4. I noticed Mark's recent updates, eg. commit: r1458187 to
tomcat/trunk/java/org/apache/tomcat/util/http/fileupload/util/mime/Base64Decoder.java,
and Konstantin's comments. I wonder how many different Base64 decoders are used
by tomcat? http://commons.apache.org/proper/commons-codec/ Impetus section
states there were once 34 different Base64 implementations in the apache
repository. Perhaps this is a good time to look at consolidation, or even
return to my earlier suggestion to define the commons jar as a dependency of
tomcat?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
