Thanks for all the help while I was developing the new test case for
https://issues.apache.org/bugzilla/show_bug.cgi?id=53584. The thread on
the users mailing list is called "AuthenticatorBase
setChangeSessionIdOnAuthentication without cookies".
I now have two new test cases working successfully against a
recently-updated trunk. I hope to use them in future as boilerplates, to
expand the set of tests, and also to examine SSO behaviour.
Before I open a bugzilla enhancement request and submit my patch files,
I would like to discuss my implementation decisions in general, to
ensure that my effort, and other developer's time, isn't wasted.
I found it necessary to modify both
org.apache.catalina.authenticator.FormAuthenticatorTest, and its parent
class org.apache.catalina.startup.SimpleHttpClient.
To save you looking it up, here is the unchanged class comment to
SimpleHttpClient:
/**
* Simple client for unit testing. It isn't robust, it isn't secure and
* should not be used as the basis for production code. Its only purpose
* is to do the bare minimum for the unit tests.
*/
FormAuthenticatorTest doesn't have a class-level comment at all, but I
have written a new one based on the conclusions in my thread on the
users list.
I would have preferred to make fairly localised changes to both of these
classes, but the existing logic seems to incorporate some fundamental
assumptions that made my intention too difficult to implement.
I am not at all proud of my code, but I believe it a) works, b) is
fairly harmonious with the existing design, and c) is amenable to the
extensions I intend to add in due course.
Firstly, I've written several small blocks of parser logic for urls and
http headers, which are specific to the test environment and not very
pretty. I looked for suitable parsers to re-use in the various tomcat
utils packages, but haven't found them yet, even though tomcat MUST be
doing similar work in an elegant and robust manner. I haven't found any
unit tests, either.
Then, I looked at the apache HttpClient project. An ideal solution would
have been to use the parsers from that project, or perhaps even the
entire client. This would have required starting with a blank sheet of
paper, and I am very reluctant to take that approach. I might have been
swayed if I had found HttpClient already in use by other tomcat unit
tests, but I couldn't find it in the dependencies and didn't want to add
a new one.
Next, the current version only supports cookies, so I had to add some
extra boolean arguments to control the use of server and client cookies
in each test case. In my professional work, I would have use singleton
inner classes to achieve type-safety and make these crucial arguments
self-documenting, but this wouldn't be compatible with the existing
style of the various current authenticator unit test classes. Also, I
wanted to make my initial change as transparent as possible, so it could
be reviewed (and accepted) with as little effort from others as possible.
I didn't want to touch SimpleHttpClient at all, but that turned out to
be unavoidable. This class does most of the http header processing, and
so it seemed ugly to split this work between the two classes. On the
other hand, all the url handling is done by FormAuthenticatorTest, so it
felt ugly to start doing any of that work in SimpleHttpClient. The
consequence is that the two classes need to be cross-wired to some
extent. This was always the case, but I had to cross some more wires for
the new test cases.
So, to summarise: I would like to make quite a bulky change to these two
classes. I am somewhat embarrassed by my ugly style and DIY approach to
parsing. Many of the line-changes in the patch are trivial, but a lot of
lines will be hit at once. I haven't gone mad with comments, but have
tried to add useful tips when an existing section of uncommented code
didn't make sense to me. On the other hand, to maintain similar
look'n'feel, I haven't been heavy-handed with comments in my new code.
Of course, I will make sure it passes checkstyle before I actually cut
the patches!
To put things in perspective, the tests only demonstrate that Mark's fix
works - and that wasn't even in question. However, I'd like to get my
change committed soon, so that I can move forward with additional test
cases.
What do you think? Should I publish and be damned, or would you like me
to do more work to eliminate some of my compromises?
Thanks for reading this...
Brian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
