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

Spring MVC Test framework never stores more than one cookie [SPR-13166] #17757

Closed
spring-projects-issues opened this issue Jun 25, 2015 · 17 comments
Labels
in: test Issues in the test module 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

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jun 25, 2015

Chris Beams opened SPR-13166 and commented

Scenario

An HTTP response contains multiple Set-Cookie headers, e.g.:

$ http head localhost:8080
HTTP/1.1 200 OK
[...]
Set-Cookie: JSESSIONID=58FA8366CB58657E4AD0AE50809D63CB; Path=/; HttpOnly
Set-Cookie: XSRF-TOKEN=27d3cc6d-6d9f-4b50-9d3c-adb67e41639e; Path=/

Problem

Spring MVC Test only stores the last cookie seen:

http.perform(get("/"))
    .andDo(print())
    .andExpect(status().isOk())
    .andExpect(cookie().exists("XSRF-TOKEN")) // PASS
    .andExpect(cookie().exists("JSESSIONID")) // FAIL
;

The output from the print() above tells the tale:

MockHttpServletResponse:
              Status = 200
       Error message = null
             Headers = {...}
        Content type = null
                Body = 
       Forwarded URL = index.html
      Redirected URL = null
             Cookies = [javax.servlet.http.Cookie@3ccd3277]

Note that the Cookies array contains only a single entry, where it should have two. (and it would be nice by the way, if the cookie entries themselves were rendered in a friendlier way).


Affects: 4.2 RC1

Issue Links:

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Hi Chris,

How are you setting the cookies in the response?

If one of them is being set by a Servlet Filter, have you added the filter to the filter chain for MockMvc?

Cheers,

Sam

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

FYI: there is nothing wrong with MockHttpServletResponse with regard to support for multiple cookies. MockHttpServletResponse.addCookie() properly adds additional cookies:

public void addCookie(Cookie cookie) {
     Assert.notNull(cookie, "Cookie must not be null");
     this.cookies.add(cookie);
}

Thus, the problem must lie within the internals of Spring MVC Test (unless you forgot to add a required Filter ;) ).

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Regarding the rendering of the cookies in print(), they're ugly simply because Cookie in javax.servlet-api-3.0.1.jar does not implement toString(); however, I'm sure we could write some custom code to make it prettier. ;)

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

Thanks Sam. The cookies in question are JSESSIONID (which I've renamed to "session") and the one set by Spring Security's csrf support (which I've renamed to "csrf"). Take a look at this gist: https://gist.github.com/cbeams/f3c3e3002118b44694b4#file-tomcatconfigtests-java-L25-L27

Notice that I'm using apply(springSecurity()) properly, and the csrf cookie does get found. However, neither the "JSESSONID" nor "session" cookies are found.

Both of these cookies are set by filters that Spring Security manages. What else would there be for me to do?

Notice the order of the cookie headers coming back:

$ http head localhost:8080
HTTP/1.1 200 OK
[...]
Set-Cookie: session=94FC8625AC6FFF3EAD6231D8B4037813; Path=/; HttpOnly
Set-Cookie: csrf=0147f666-411d-4399-bccb-bd73490c68c2; Path=/
[...]

Notice that csrf is the last one set. It sure seems like that is ending up overwriting the "session" cookie in a "last-one-wins" fashion (as opposed to an "append it to the list of cookies" fashion).

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Interesting.

Out of curiosity, what happens if you execute something like the following in your test (excluding the expectation that currently fails)?

ServletRequestAttributes requestAttributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
MockHttpServletResponse mockResponse = (MockHttpServletResponse) requestAttributes.getResponse();
assertEquals(2, mockResponse.getCookies().length);

Or... if you inspect the contents of that getCookies() array, what does it contain?

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

The updated test:

    @Test
    public void shouldSetCustomizedSessionCookie() throws Exception {
        http.perform(get("/"))
            .andDo(print())
            .andExpect(status().isOk())
            .andExpect(cookie().exists("csrf"))
            .andExpect(cookie().doesNotExist("JSESSIONID"))
            //.andExpect(cookie().exists("session"))
        ;

        ServletRequestAttributes requestAttributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
        MockHttpServletResponse mockResponse = (MockHttpServletResponse) requestAttributes.getResponse();
        assertEquals(2, mockResponse.getCookies().length);
    }

And the result:

expected:<2> but was:<0>
Expected :2
Actual   :0

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

What Spring MVC Test does to access cookies is dead simple see here. I think the issue is not related to access or asserting expectations. There is something else going on where the expected cookie is not added when running with Spring MVC Test. To confirm I just did a simple test with a controller adding two cookies and I had no trouble asserting that both exist with cookie().exists.

/cc robwinch

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Thanks for updating the test, Chris.

I just wanted to rule out the possibility that one of the cookies was being set via the RequestAttributes - which could cause such a cookie not to be seen by Spring MVC Test.

So your test in fact rules that out.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Chris, in the original comment "jsessionid" is missing. In fact I don't see what would set that since this is not running in a Servlet container. However in the referenced gist the code is asserting the opposite that "jsessionid" doesn't exist, which makes more sense but then you're checking "session". Can you clarify? Regarding "jsessionid" I presume the gist is the correct version. Also what is the "session" cookie and what's supposed to set that?

@spring-projects-issues
Copy link
Collaborator Author

Rob Winch commented

Thanks for the report Chris.

I created two instances of a CookieFilter (which adds a cookie to the response) and added them to Spring Security's Filters. When running the test with MockMvc it seems to work for me. See rwinch/spring-security-sample/SRP-13116

Like Rossen, I'm curious about the JSESSIONID vs the session cookie differences. I'm also wondering are you using Spring Session? Spring Session is added outside of Spring Security, so you would need to add that to MockMvc separately.

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

Ok, it looks like I'm confused about the way things work with MVC Test. Rossen, the "session" cookie is Tomcat's usual JSESSIONID cookie, just renamed:

    @Configuration
    static class TomcatConfig {

        @Bean
        public EmbeddedServletContainerFactory tomcat() {
            TomcatEmbeddedServletContainerFactory tomcat = new TomcatEmbeddedServletContainerFactory();
            tomcat.addContextCustomizers(context -> context.setSessionCookieName("session"));
            return tomcat;
        }
    }

The source of my confusion is summed up by your quote above: "... this is not running in a Servlet container". I actually thought it was wiring everything up with the same embedded tomcat configuration that is in use at actual runtime. Since that is not the case, this issue is invalid. Feel free to close it.

Perhaps these limitations could be mentioned a bit more prominently in the documentation? The current docs do mention that MVC Test makes end-to-end testing "without needing to run a servlet container", but it also says " For the most part everything should work as it does at runtime with the exception of JSP rendering, which is not available outside a Servlet container."

Assumptions about session cookies would be another "something that doesn't work", as well as the workarounds necessary to apply(springSecurity()) and any other filters that need to be manually applied.

I guess the point is that the way MVC Test "feels" to the user is that they really are working against a fully wired up webapp. It "feels" like the container is there, even though it's not. This means that these assumptions about all the usual filters and cookies, etc being there are natural to make, and a surprise every time they're violated. An early heads-up to check those assumptions at the door might be helpful.

In any case, thanks everyone for the quick responses!

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jun 26, 2015

Sam Brannen commented

Ok, Chris, we'll close this issue and create follow-up tickets to improve the documentation on the limitations of Spring MVC Test (see #17760) and to improve the debug output for cookies (see #17759).

I actually thought it was wiring everything up with the same embedded tomcat configuration that is in use at actual runtime.

I think you are possibly confusing Spring MVC Test's out-of-container testing support with Spring Boot's in-container testing support.

Using @WebAppConfiguration with the Spring TestContext Framework loads a WebApplicationContext for your integration test, but that WebApplicationContext is not running in a Servlet container. Instead, Spring's Servlet Mocks are used to simulate the container.

In contrast, with Spring Boot... if you use @WebIntegrationTest (or @IntegrationTest with @WebAppConfiguration)... then you will in fact have your WebApplicationContext running in an embedded Servlet container (i.e., Tomcat, Jetty, or Undertow). Note, however, that with in-container tests using Spring Boot you'll likely want to perform actual HTTP requests to the embedded container via RestTemplate (or Spring Boot's TestRestTemplate), Selenium, etc.

Make sense?

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

Thanks Sam—I actually hadn't worked with @WebIntegrationTest yet, will check it out. And am now watching that documentation issue.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Okay thanks for the feedback. Improving the documentation on that point, perhaps even mentioning Spring Boot's @WebIntegrationTest alternative makes sense. The easiest way to think about this is having a blank MockHttpServletRequest to start. Whatever you add to it is it. The things that may catch you out are the jsessionid cookie, the context path, the lack of forwarding or error or async dispatches, and jsp rendering are the main ones I can think of.

The question of Spring MVC Test vs @WebIntegrationTest is an interesting one. There are pros and cons to each. For once these are different stops on the scale from pure unit to full integration tests. Well, none of these is pure unit testing but Spring MVC Test is a little closer to unit testing. You can for example isolate the service layer with mocks injected into the controllers and then you're testing the web layer only with Spring config. You could also be using the standalone setup focusing on the controller and manually providing the configuration.

Also with Spring MVC Test you're conceptually on the inside on the server-side so we know what handler was used, if an exception was handled with a HandlerExceptionResolver, we have access to the model, etc. It means you can make assertions on the kinds of things you couldn't when using a REST client in which case the server is a black box. Or you might say that one should avoid looking at anything but the response, otherwise the tests are fragile. I think there is room here for multiple styles and schools of thought.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

All very valid points, Rossen, and ...

I think there is room here for multiple styles and schools of thought.

Yes... I couldn't agree more. ;)

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jun 27, 2015

Sam Brannen commented

Chris Beams, I baked you some pretty cookies in #17759.

Is that better now? ;)

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

Very nice! Thanks.

@spring-projects-issues spring-projects-issues added type: bug A general bug status: declined A suggestion or change that we don't feel we should currently apply in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jan 11, 2019
@spring-projects-issues spring-projects-issues removed the type: bug A general bug label Jan 12, 2019
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: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

1 participant