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

Non-value attributes in Set-Cookie header #1171

Closed
nnposter opened this issue Apr 2, 2018 · 3 comments
Closed

Non-value attributes in Set-Cookie header #1171

nnposter opened this issue Apr 2, 2018 · 3 comments

Comments

@nnposter
Copy link

nnposter commented Apr 2, 2018

Parser for Set-Cookie header (function parse_set_cookie in http.lua) is currently assigning attributes without a value (such as Secure or HttpOnly) an artificial Boolean value of true (not string "true"). In contrast, attributes with explicitly empty values (...; attr=;...) are assigned a zero-length string as the value.

According to RFC 6265, section 5.2 these two cases should have the same outcome: empty value.

I am proposing to change the parser to assign a zero-length string to value-less attributes, which would have the following effect:

Pro

  • A parsed attribute always has a value of type string
  • Zero-length string is still interpreted as true in logical tests so this change should have negligible impact on existing code
  • There is no distinction between ...; attr;... and ...; attr=;..., which is what the RFC prescribes.

Con

  • There is no distinction between ...; attr;... and ...; attr=;..., so response member rawheader would have to be used if somebody truly needs to differentiate between the two cases.

I will implement the change in a few weeks unless anybody brings up concerns.

@cldrn
Copy link
Member

cldrn commented Apr 3, 2018

The change seems correct. I'm just wondering if we can incorporate all the changes you've made to be more rfc complaint into @vinamrabhatia work too.

@vinamrabhatia Have you looked into nnposter's commits related to http and cookies? It would be great for you to consider them in your final revision.

@nnposter
Copy link
Author

Resolved with r37235 (23d61f5)

@vinamrabhatia
Copy link
Contributor

@nnposter I will make the similar changes in the cookies library work too.

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

No branches or pull requests

3 participants