Skip to content

Merge Expires and Max-Age attributes #33369

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Merge Expires and Max-Age attributes #33369

wants to merge 1 commit into from

Conversation

imvtsl
Copy link
Contributor

@imvtsl imvtsl commented Aug 12, 2024

org.springframework.http.client.reactive.ClientHttpResponse and its implementations don't process Expires Cookie attribute. However, they only process Max-Age attribute.

Logic defined in RFC 6265. Point 3 of section 5.3 in this RFC gives higher precedence to Max-Age over Expires. This change follows this RFC and does the following:
1- If both Max-Age and Expires attribute are present in the cookie, set maxAge field in ResponseCookie to Max-Age attribute.
2- If only Expires attribute is present in the cookie, set maxAge field in ResponseCookie to Expires attribute.
2- If only Max-Age attribute is present in the cookie, set maxAge field in ResponseCookie to Max-Age attribute.

Closes #33157

@imvtsl imvtsl changed the title Merge expires and Max-Age attributes Merge Expires and Max-Age attributes Aug 12, 2024
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 12, 2024
@snicoll snicoll added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Aug 12, 2024
@imvtsl imvtsl force-pushed the issue-33157 branch 3 times, most recently from b03e21c to 122d79a Compare August 13, 2024 01:34
@imvtsl imvtsl marked this pull request as ready for review August 13, 2024 02:04
@imvtsl
Copy link
Contributor Author

imvtsl commented Aug 13, 2024

@sbrannen please review.

@imvtsl imvtsl mentioned this pull request Aug 13, 2024
Closed
GovindarajanL
GovindarajanL reviewed Aug 14, 2024
View reviewed changes
Copy link

@GovindarajanL GovindarajanL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey added few review comments , kindly check

@sbrannen sbrannen self-assigned this Aug 23, 2024
@sbrannen
Copy link
Member

Thanks for the PR, @imvtsl!

I'll take a look at it in the coming days.

@bclozel bclozel assigned bclozel and unassigned sbrannen Sep 13, 2024
@bclozel
Copy link
Member

bclozel commented Sep 13, 2024

Sorry for the late feedback @imvtsl
In the end, we went with a simpler approach in #33157 as this PR was creating new public API and the tests were too involved.

Thanks for your contribution!

@bclozel bclozel closed this Sep 13, 2024
@bclozel bclozel added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GovindarajanL GovindarajanL GovindarajanL left review comments

Assignees

@bclozel bclozel

Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Reactive HttpComponentsClientHttpResponse ignores Expires cookie attribute
6 participants
@imvtsl @sbrannen @bclozel @GovindarajanL @snicoll @spring-projects-issues