Skip to content

Preserve expires attribute in MockCookie #23769

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

Closed
wants to merge 1 commit into from

Conversation

vpavic
Copy link
Contributor

@vpavic vpavic commented Oct 8, 2019

At present, MockCookie doesn't preserve expires attribute. This has a consequence that cookie value set using MockHttpServletResponse#addHeader containing expires attribute will not match the cookie value obtained from MockHttpServletResponse#getHeader since the expires will get calculated based on current time.

This is very simple to demonstrate using a test that's added to MockHttpServletResponseTests as a part of this PR:

@Test
void addCookieHeaderWithExpires() {
    String cookieValue = "SESSION=123; Path=/; Max-Age=100; Expires=Tue, 8 Oct 2019 19:50:00 GMT; Secure; " +
            "HttpOnly; SameSite=Lax";
    response.addHeader(HttpHeaders.SET_COOKIE, cookieValue);
    assertThat(response.getHeader(HttpHeaders.SET_COOKIE)).isEqualTo(cookieValue);
}

This PR enhances MockCookie to preserve expires attribute.

At present, MockCookie doesn't preserve expires attribute. This has a consequence that cookie value set using MockHttpServletResponse#addHeader containing expires attribute will not match the cookie value obtained from MockHttpServletResponse#getHeader since the expires will get calculated based on current time.

This commit enhances MockCookie to preserve expires attribute.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 8, 2019
@vpavic
Copy link
Contributor Author

vpavic commented Oct 28, 2019

Any feedback on this? The current behavior, illustrated by test in the opening comment, is effectively a bug.

@sbrannen sbrannen added in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) labels Oct 28, 2019
@sbrannen sbrannen added this to the 5.2.1 milestone Oct 28, 2019
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.1.x labels Oct 28, 2019
@sbrannen
Copy link
Member

Sorry for the delayed response. This one slipped under the radar.

It looks legit to me, so we'll try to get this in for 5.1.11 and 5.2.1.

@sbrannen sbrannen added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 28, 2019
@sbrannen sbrannen self-assigned this Oct 28, 2019
@sbrannen
Copy link
Member

For future reference, please wrap commit message descriptions at 72 characters.

@sbrannen
Copy link
Member

Also, the imports in MockCookieTests broke the build. So please be sure to execute ./gradlew check before submitting a PR.

@sbrannen sbrannen closed this in 9b20876 Oct 29, 2019
sbrannen added a commit that referenced this pull request Oct 29, 2019
sbrannen pushed a commit that referenced this pull request Oct 29, 2019
At present, MockCookie doesn't preserve expires attribute. This has a
consequence that a cookie value set using
MockHttpServletResponse#addHeader containing an expires attribute will
not match the cookie value obtained from
MockHttpServletResponse#getHeader, since the expires attribute will get
calculated based on current time.

This commit enhances MockCookie to preserve the expires attribute.

Closes gh-23769
sbrannen added a commit that referenced this pull request Oct 29, 2019
@vpavic vpavic deleted the improve-mockcookie branch October 29, 2019 13:42
@vpavic
Copy link
Contributor Author

vpavic commented Oct 29, 2019

Thanks for taking care of this @sbrannen! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants