Skip to content

Update Cookie headers in MockHttpServletRequest and Response [SPR-15225] #19790

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
spring-projects-issues opened this issue Feb 6, 2017 · 5 comments
Assignees
Labels
in: test Issues in the test module type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Feb 6, 2017

Andy Wilkinson opened SPR-15225 and commented

When setContentType is called on MockHttpServletRequest or MockHttpServletResponse the Content-Type header is automatically updated. However, when setCookies is called on MockHttpServletRequest or addCookie is called on MockHttpServletResponse the respective Cookie and Set-Cookie headers are unaffected.

In the interests of consistency, I'd like setCookies and addCookie to automatically update the Cookie and Set-Cookie headers respectively. Setting the Cookie header would also provide an opportunity to apply the recommended ordering:

2.  The user agent SHOULD sort the cookie-list in the following
    order:

    *  Cookies with longer paths are listed before cookies with
       shorter paths.

    *  Among cookies that have equal-length path fields, cookies with
       earlier creation-times are listed before cookies with later
       creation-times.

Affects: 4.3.6

Issue Links:

Referenced from: commits 6e71828, e33f603, e5fc40a

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Feb 23, 2017

Rossen Stoyanchev commented

In the case of Content-Type we parse it in order to set the character encoding field. From there setting the header is an easy extra step. It's also symmetrical in that you can change the header or call setContentType and the changes will propagate.

The case of cookies is somewhat different. It's a bit more effort to format the cookie as a String. We can certainly do that but I don't think it's worth going as far as being able to parse it from a header to make it symmetrical. So I'd like to double check what the underlying motivation for this is. Purely from a testing perspective checking the cookies should be good enough. If this relates to REST Docs, I can see the value in updating from Cookie -> headers but not the other way around.

Also I'm wondering if you care about #19773 which is related.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Feb 28, 2017

Andy Wilkinson commented

This does indeed relate to REST Docs. Updating from Cookie -> Headers is where I see the value too. I don't see any value in the other way around either. Sorry if I gave a different impression above. Perhaps something I wrote was ambiguous?

Also I'm wondering if you care about #19773 which is related.

I don't care about it from a REST Docs perspective, but I do from a general consistency of behaviour perspective. I'd welcome all the setters that relate to a header automatically updating the header when they're called.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

No you didn't ask for it. I was simply clarifying since seeking consistency with content-type handling also implies two way handling between headers and Servlet API types. I was simply making sure that wasn't part of your expectations.

@spring-projects-issues
Copy link
Collaborator Author

Andy Wilkinson commented

My understanding of RFC-6265 is that there should be a space between the ; and an attribute name, i.e. the header should be something like name=value; Domain=localhost; HttpOnly rather than name=value;Domain=localhost;HttpOnly. The changes made for this issue produce the latter. FWIW, a change was recently made to Tomcat so that it now produces the former: https://bz.apache.org/bugzilla/show_bug.cgi?id=60876. I wonder if it's worth aligning with that in Spring Framework?

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Thanks Andy, should be fixed now.

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 type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants