On 04/01/13 19:58, Konstantin Kolinko wrote:
2013/1/4 Brian Burch <br...@pingtoo.com>:
On 29/11/12 16:11, Brian Burch wrote:
On 29/11/12 14:37, bugzi...@apache.org wrote:
https://issues.apache.org/bugzilla/show_bug.cgi?id=54190
Re: [Bug 54190] TestNonLoginAndBasicAuthenticator does not test session
timeout properly
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--- Comment #6 from Mark Thomas <ma...@apache.org> ---
<snip/>
If you start looking at the TODOs, I suggest you take a look at
org.apache.tomcat.util.http.parser.HttpParser#parseAuthorizationDigest()
I suspect a new parseAuthorizationBasic() method is the way to go as that
should handle the various whitespace issues noted.
Noted. Assume that I will look into it unless you hear otherwise. I
won't open a bug against BasicAuthenticator yet.
My motive is the original thread above, but my question has a wider scope. I
have started a new thread, even if it doesn't run far, just to make it
easier for others to find.
I started looking at Mark's suggestion and got a long way with the change.
Currently, BasicAuthenticator.authenticate uses
org.apache.catalina.util.Base64.decode(ByteChunk,CharChunk).
I decided to follow the pattern used by DigestAuthenticator and create a new
method
org.apache.tomcat.util.http.parser.HttpParser.parseAuthorizationBasic(StringReader).
Once I got there, I discovered that org.apache.catalina.util.Base64 doesn't
currently have any methods to decode a StringReader or String object.
It didn't take me long to find
org.apache.commons.codec.binary.Base64.Base64, which has all the right
methods to make my own change lightweight and in need of fewer unit tests.
I then started hacking the build to use the commons Base64 jar as an
external dependency. Unfortunately, that turned out to be more difficult
than I expected, and I haven't yet finished the job.
I had a lot of non-tomcat priorities and have only just gone back to my
work-in-progress. Before I go any further, I would like to find out what the
general policy is in this sort of situation:
1. we have code that uses a tomcat-specific utility class that has been
stable for a long time.
2. The function of that class is similar, but not identical, to that of one
in apache commons.
Should we:
a) extend the tomcat-specific utility class to provide methods for new logic
that is not related to an important bugfix (effectively increasing the
duplication of a commons class)
or
b) use the commons class for the new logic, but leave everything else
unchanged (effectively having two classes performing similar tasks)
or
c) use the commons class and eliminate any redundant logic in
tomcat-specific classes (our class wraps the commons class to provide extra
functionality but no duplication of core logic)
I'm happy to go in any of these 3 directions. My preference would be to use
(b) initially, and follow up with (c) soon after.
Is there a policy, or a generally held opinion?
The general rule is that Tomcat cannot directly use the commons
library, as it might clash with such use in a web application. (Though
if you need them for tests only, such a clash should not be a matter).
Whenever we need classes from commons, we copy them into our source
tree into a different package (usually with "svn copy" to preserve
history).
Mark usually removes unused code from the copy, leaving only those
bits that are actually used by Tomcat.
Examples:
org.apache.tomcat.util.http.fileupload
org.apache.tomcat.util.bcel
org.apache.tomcat.util.digester
Once I got there, I discovered that org.apache.catalina.util.Base64 doesn't
currently have any methods to decode a StringReader or String object.
HTTP headers are converted from bytes to characters using ISO-8859-1
encoding. (Actually they should be 7-bit ASCII). This conversion is
reversible.
If all you need it for is a test method, I think I would just convert
the string to bytes and use the existing API. Do you need to support
a Reader there?
In fact, the change is not aimed at test code - it will be used by
BasicAuthenticator, although there will also be new tests for the parser
method too.
Thanks very much for your helpful explanation. I understand the reasons,
but I had not thought about the problem that way before - it is almost
the opposite of my intuition. I had thought the dependent jars section
of the tomcat build was a kind of prototype of maven dependencies, and
so assumed that more external dependencies would be a good thing because
it would eliminate duplicate code. It is interesting to be shown a valid
counter-argument.
I will back out the work I've done already to use commons, and proceed
with an extension to the org.apache.catalina.util.Base64 class.
Regards,
Brian
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
