-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Updates Cookie class to be more generic in attribute parsing and emit. #521
Conversation
More discussion can be found in #517, which is now closed. Please continue the discussion here. |
26468b8
to
455787a
Compare
This is so the library can age better as new attributes are added to RFC revisions.
1. Made Cookie Name and Value properties case insensitive 2. Throws exception on illegal Cookie Name 3. Doesn't emit "false" flag values 4. Properly escape key-value attributes.
455787a
to
d334b58
Compare
cookie-keys are not case sensitive, but json-keys are.
@Hubgut, please review the latest changes. I believe I corrected the escaping issue and also a few other minor tweeks |
Unit tests fail for Maven with Java 7 but pass for Gradle with Java 8. The failures are in unrelated modules, due to the unit tests not accounting for possible differences in JSONObject ordering. |
@johnjaylward. Now, I found some time for my little project and changed to the most current version. |
What problem does this code solve?
See #517
Brings the Cookie implementation closer to the RFC specification (https://tools.ietf.org/html/rfc6265)
This is so the library can age better as new attributes are added or removed in future RFC revisions.
This also fixes bugs in the implementation:
Risks
Low to Medium. I doubt many people were using this since the problems brought up in #517 were not found until now. They seem to be pretty glaring gaps in the parsing and emit logic.
Changes to the API?
No
Will this require a new release?
Yes
Should the documentation be updated?
I don't think it's needed
Does it break the unit tests?
Yes, corrections were made
Was any code refactored in this commit?
No, bug fixes and support for new attributes only.
Review status
APPROVED
Starting 3 day comment window