Fix #9468 Space in Cookie name #9471
Conversation
Added a violation to allow unquoted spaces in cookie values Signed-off-by: gregw <gregw@webtide.com>
Added a violation to allow unquoted spaces in cookie values Signed-off-by: gregw <gregw@webtide.com>
| @@ -74,6 +74,8 @@ public static Stream<Arguments> data() | |||
| Arguments.of("abc= \"x\" ", "abc", "x"), | |||
| Arguments.of("abc= \"x\" ;", "abc", "x"), | |||
| Arguments.of("abc= \"x\" ; ", "abc", "x"), | |||
| Arguments.of("abc = x y z ", "abc", "x y z"), | |||
| Arguments.of("abc = x y z ", "abc", "x y z"), | |||
There was a problem hiding this comment.
Can you add a test case that also uses a value with token separators (in this case a : colon)?
Arguments.of("abc=a:x b:y c:z", "abc", "a:x b:y c:z")There was a problem hiding this comment.
What would happen if the input was abc=a;x b;y c;z as well? (semi-colon)
Signed-off-by: gregw <gregw@webtide.com>
| /** | ||
| * Allow spaces within values without quotes. | ||
| */ | ||
| SPACE_IN_VALUES("https://www.rfc-editor.org/rfc/rfc6265#section-5.2", "Space in value"); |
There was a problem hiding this comment.
Rename to singular, SPACE_IN_VALUE.
There was a problem hiding this comment.
@sbordet Most of the violations are in the plural. It is more natural to say you allow the plural than to allow the singular.
There was a problem hiding this comment.
maybe should be SPACES_IN_VALUES?
jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java
Show resolved
Hide resolved
| { | ||
| spaces++; | ||
| } | ||
| else if (c == ';' || c == ',' || c == '\t') |
There was a problem hiding this comment.
Regardless of the comment above, I don't think we can treat \t as a separator between cookies, can we?!?
There was a problem hiding this comment.
We don't. This just changes state to END, which can accept a tab as optional white space around a value, but not as a separator.
There was a problem hiding this comment.
Not sure I follow. If we encounter \t the value ends, so it is a terminator char for the value, hence a separator between cookies, like the other chars in the if statement.
\t is a control char, so it's forbidden in every RFC, isn't it?
Why do we treat it specially? Historical reasons?
jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserLenientTest.java
Show resolved
Hide resolved
Signed-off-by: gregw <gregw@webtide.com>
Signed-off-by: gregw <gregw@webtide.com>
| { | ||
| spaces++; | ||
| } | ||
| else if (c == ';' || c == ',' || c == '\t') |
There was a problem hiding this comment.
Not sure I follow. If we encounter \t the value ends, so it is a terminator char for the value, hence a separator between cookies, like the other chars in the if statement.
\t is a control char, so it's forbidden in every RFC, isn't it?
Why do we treat it specially? Historical reasons?
|
@sbordet tab is treated specially as we have a violation that allows optional white space before/after a value. RFC6265 defines OWS in terms of WSP, which in RFC5234 is space or tab. Hence when we extended where OWS could be in the violation, we also included tab. In this PR, we are allowing a space to be in the value, which is different to OWS, hence the decision to not include tab. |
Added a violation to allow unquoted spaces in cookie values to fix #9468