Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements for Set-Cookie parsing, allow lax parsing of older specs #2368

Merged
merged 3 commits into from Sep 23, 2022

Conversation

idelpivnitskiy
Copy link
Member

Motivation:

  • Improve parsing of Set-Cookie headers #2329 made parsing of set-cookie header strict according to RFC6265. In practice, there are still many implementations that encode cookies according to the obsolete RFC2965 and/or RFC2109.
  • Semicolon and space are not validated after a wrapped value.
  • Without a cookie name in the exception message it's harder to find a problematic cookie.

Modifications:

  • Allow no space after semicolon by default;
  • Add a system property io.servicetalk.http.api.headers.cookieParsingStrictRfc6265 to enforce strict parsing;
  • Instead of blindly skipping SEMI and SP after DQUOTE, validate skipped characters;
  • Include the cookie name (if already parsed) in all exception messages;
  • Enhance test coverage for DefaultHttpSetCookie#parseSetCookie;

Result:

  1. No space is required after semicolon by default.
  2. Characters after a wrapped value are validated.
  3. Exception messages include a cookie name when possible.
  4. More test coverage.

Motivation:

- apple#2329 made parsing of `set-cookie` header strict according to RFC6265.
In practice, there are still many implementations that encode cookies
according to the obsolete RFC2965 and/or RFC2109.
- Semicolon and space are not validated after a wrapped value.
- Without a cookie name in the exception message it's harder to find a
problematic cookie.

Modifications:
- Allow no space after semicolon by default;
- Add a system property
`io.servicetalk.http.api.headers.cookieParsingStrictRfc6265` to enforce
strict parsing;
- Instead of blindly skipping `SEMI` and `SP` after `DQUOTE`, validate
skipped characters;
- Include the cookie name (if already parsed) in all exception messages;
- Enhance test coverage for `DefaultHttpSetCookie#parseSetCookie`;

Result:

1. No space is required after semicolon by default.
2. Characters after a wrapped value are validated.
3. Exception messages include a cookie name when possible.
4. More test coverage.
@idelpivnitskiy idelpivnitskiy merged commit ae4d6d1 into apple:main Sep 23, 2022
@idelpivnitskiy idelpivnitskiy deleted the set-cookie branch September 23, 2022 20:14
chrisvest added a commit to chrisvest/netty that referenced this pull request Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants