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

Inconsistencies with Cookie value encoding #690

Closed
FabianFrank opened this issue Oct 31, 2018 · 0 comments
Closed

Inconsistencies with Cookie value encoding #690

FabianFrank opened this issue Oct 31, 2018 · 0 comments
Assignees
Labels

Comments

@FabianFrank
Copy link

In ktor 0.9.5 (and Cookie.kt on master as of right now https://github.com/ktorio/ktor/blob/06a0c5e70688277df32dcd6c7a6deff30561409d/ktor-http/src/io/ktor/http/Cookie.kt), there seems to be a discrepancy between what ktor allows as a raw value and the raw values it produces. The = sign is considered to not be allowed in a raw cookie value:

private val cookieCharsShouldBeEscaped = setOf(';', ',', '=', '"')
private fun Char.shouldEscapeInCookies() = this.isWhitespace() || this < ' ' || this in cookieCharsShouldBeEscaped

but when encoding a cookie value with CookieEncoding.BASE64_ENCODING a Base64 variant is chosen that will pad with =:

CookieEncoding.BASE64_ENCODING -> value.encodeBase64()

I think it would be better if ktor would either choose to allow = in cookie values in general or as an alternative use a Base64 encoding variant that does not produce characters that are disallowed in cookie values according to shouldEscapeInCookies(). Personally I think it would be best to allow = in cookie values (but not names), because a lot of existing systems are doing so and the way I understand https://tools.ietf.org/html/rfc6265#section-4.1.1 = is still allowed in cookie values. I'm happy to contribute a PR that would change the code to apply different character sets if we agree that this is the right thing to do.

@e5l e5l self-assigned this Nov 2, 2018
jonasbark pushed a commit to jonasbark/ktor that referenced this issue Jan 16, 2019
* commit 'a7b406cf34cbaf1a767d226701003965d9f2740a': (28 commits)
  Improve HTTP client engine container documentation
  Add tailcard prefix support (ktorio#876, ktorio#526)
  Fix semaphore leave(locked coroutine without thread)
  Fix CIO headers parsing
  Fix client websocket nonce size
  Fix scheme parsing
  Await skipCancel in CIO client response pipeline
  Introduce default ktor exceptions and parameters delegation support
  Add word to dictionary
  Pull application property to ApplicationEngine interface
  Fix loosing errors in StatusPages if there was already a response sent
  Apply workaround to kotlin gradle MPP plugin bug
  Prohibit settings session after responding call
  Client: Support relative redirects by making #takeFrom() resolve relative urls. (ktorio#849)
  Add base64 cookies tests (related to ktorio#690)
  Move NonceManager to ktor-utils
  Reformat code according to the new codestyle
  Introduce NonceManager instead of OAuth2StateProvider
  Introduce withReplacedParameter on HttpAuthHeader
  Fix concurrent MessageDigest usage in server digest auth provider
  ...
e5l added a commit that referenced this issue Feb 20, 2019
e5l added a commit that referenced this issue Feb 21, 2019
e5l added a commit that referenced this issue Feb 25, 2019
e5l added a commit that referenced this issue Feb 25, 2019
@e5l e5l closed this as completed in 6e79f4d Feb 26, 2019
schleinzer pushed a commit to schleinzer/ktor that referenced this issue Feb 26, 2019
schleinzer pushed a commit to schleinzer/ktor that referenced this issue Feb 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants