Created attachment 25190 [details] Code patch that stops the loop parsing the final character if it's an equal sign Cookie values such as the following don't get their last value parsed as an empty value. i.e. this does create a 'bob' cookie key: Cookie: fred=1; jim=2; bob and this doesn't create a 'bob' cookie key: Cookie: fred=1; jim=2; bob=
Created attachment 25191 [details] Unit test patch
Hmm. Neither of those two cookies are valid since both name and value are mandatory. Tomcat 6 (and probably earlier) has allowed what is referred to in the code as name only cookies. I think this is another candidate for a cookie configuration option in Tomcat 7 (ALLOW_NAME_ONLY_COOKIES ?) that defaults to false and rejects these invalid cookies. In terms of what this option does allow, since we are outside the spec, we can choose what we want to allow and disallow. I quite like the current allow 'name' but not 'name=' as the latter looks like an error whilst the first looks an attempt to have a name only cookie.
I've definitely seen the appended = sign coming from IE6, IE7 and IE8 clients, both within the body of the Cookie text (where Tomcat handles things) and as the last value (where Tomcat drops the value). I don't know where Tomcat draws the line between spec beauty and browser reality; I agree with your comment if it's the former.
What are the use cases for these invalid cookies? What functionality are they supporting?
I don't know - I see them coming from IE in logs and, I'm assuming, it's IE deciding the format of how to send them. Use case - not something I can figure out.
Henri, what is the behavior of TC when these invalid cookies are received from a client?
It doesn't create the last cookie value.
That seems like reasonable behavior to me. If there were some kind of exception or the request was not processed, I'd think this was a bug. In this case, I believe Tomcat is spec-compliant. If you can provide some research suggesting that this is a common MSIE phenomenon, I'll consider writing a patch for it.
I'm poorly explaining. Tomcat is happy with all of the following: Cookie: fred=1; jim=2; bob Cookie: fred=1; jim=2; bob; george=3 Cookie: fred=1; jim=2; bob=; george=3 It isn't happy with: Cookie: fred=1; jim=2; bob= Looking at logs, I can see that IE6, IE7 and IE8 all sends the bob= type entries. Generally this is fine, unless it's the last item in the semi-colon delimited list, in which case Tomcat drops the cookie entry. I agree with everything said - if name and value are mandatory then none of the above should be accepted. However, the current Tomcat functionality is such that 3/4 of the options are accepted and anyone relying on this is going to have a very confusing feature where IE traffic sporadically drops a cookie.
(In reply to comment #9) > Cookie: fred=1; jim=2; bob=; george=3 Tomcat is okay with that last one? Okay, then Tomcat is exhibiting inconsistent behavior, and this should be considered a bug. > Looking at logs, I can see that IE6, IE7 and IE8 all sends the bob= type > entries. Generally this is fine, unless it's the last item in the semi-colon > delimited list, in which case Tomcat drops the cookie entry. Would you agree that dropping the bob= cookie altogether no matter where it is in the list is expected, spec-compliant behavior? > I agree with everything said - if name and value are mandatory then none of the > above should be accepted. However, the current Tomcat functionality is such > that 3/4 of the options are accepted and anyone relying on this is going to > have a very confusing feature where IE traffic sporadically drops a cookie. Agreed.
Yes, bob= is fine unless it's on the end of the line and then it's dropped (I just doublechecked with some unit tests as it's been a while since I was digging into this). I think there are three options: 1) Spec purity. Drop 'key' and drop 'key='. 2) Closer to the spec. Drop 'key=', keep 'key'. 3) Backwards compat. Support 'key=' at end of line. I think that, ideally, it would be a case of TC7 being #1, while Mark's proposed ALLOW_NAME_ONLY_COOKIES feature would continue to support #3. I don't think there's a value in dropping 'key=' and keeping 'key'.
I've fixed the inconsistent handling. Name only cookies are currently always allowed. I do plan on adding an option to make accepting them configurable with a default of not accepting them.
Option added in 7.0.x and will be in 7.0.6 onwards. I also added a note to the migration guide.