From 5990548f6ffec9877889976f825782db4e2ce8cb Mon Sep 17 00:00:00 2001 From: Ilya Lukyanovich Date: Mon, 3 Jun 2019 15:02:44 +0300 Subject: [PATCH] Fix MockHttpServletRequest.setCookies to produce single cookie header Prior to this commit, MockHttpServletRequest.setCookies() produced one Cookie header per supplied cookie, resulting in multiple Cookie headers which violates the specification. This commit fixes this by ensuring that all cookie name-value pairs are stored under a single Cookie header, separated by a semicolon. Closes gh-23074 --- .../mock/web/MockHttpServletRequest.java | 21 +++- .../mock/web/MockHttpServletRequestTests.java | 6 +- .../mock/web/test/MockHttpServletRequest.java | 106 ++++++++++++------ 3 files changed, 93 insertions(+), 40 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java index c81412e40e..29810935a6 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -41,6 +41,7 @@ import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.TimeZone; +import java.util.stream.Collectors; import javax.servlet.AsyncContext; import javax.servlet.DispatcherType; import javax.servlet.RequestDispatcher; @@ -58,6 +59,7 @@ import javax.servlet.http.Part; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; +import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.LinkedCaseInsensitiveMap; @@ -951,12 +953,18 @@ public class MockHttpServletRequest implements HttpServletRequest { public void setCookies(@Nullable Cookie... cookies) { this.cookies = (ObjectUtils.isEmpty(cookies) ? null : cookies); - this.headers.remove(HttpHeaders.COOKIE); - if (this.cookies != null) { - Arrays.stream(this.cookies) - .map(c -> c.getName() + '=' + (c.getValue() == null ? "" : c.getValue())) - .forEach(value -> doAddHeaderValue(HttpHeaders.COOKIE, value, false)); + if (this.cookies == null) { + removeHeader(HttpHeaders.COOKIE); } + else { + doAddHeaderValue(HttpHeaders.COOKIE, encodeCookies(this.cookies), true); + } + } + + private static String encodeCookies(@NonNull Cookie... cookies) { + return Arrays.stream(cookies) + .map(c -> c.getName() + '=' + (c.getValue() == null ? "" : c.getValue())) + .collect(Collectors.joining("; ")); } @Override @@ -1272,6 +1280,7 @@ public class MockHttpServletRequest implements HttpServletRequest { * Otherwise it simply returns the current session id. * @since 4.0.3 */ + @Override public String changeSessionId() { Assert.isTrue(this.session != null, "The request does not have a session"); if (this.session instanceof MockHttpSession) { diff --git a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletRequestTests.java b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletRequestTests.java index 59ac0c6382..502a807592 100644 --- a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletRequestTests.java +++ b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletRequestTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -259,7 +259,9 @@ public class MockHttpServletRequestTests { assertEquals("bar", cookies[0].getValue()); assertEquals("baz", cookies[1].getName()); assertEquals("qux", cookies[1].getValue()); - assertEquals(Arrays.asList("foo=bar", "baz=qux"), cookieHeaders); + + assertEquals(1, cookieHeaders.size()); + assertEquals("foo=bar; baz=qux", cookieHeaders.get(0)); } @Test diff --git a/spring-web/src/test/java/org/springframework/mock/web/test/MockHttpServletRequest.java b/spring-web/src/test/java/org/springframework/mock/web/test/MockHttpServletRequest.java index 05a32d7aca..bdfb905190 100644 --- a/spring-web/src/test/java/org/springframework/mock/web/test/MockHttpServletRequest.java +++ b/spring-web/src/test/java/org/springframework/mock/web/test/MockHttpServletRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -41,6 +41,7 @@ import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.TimeZone; +import java.util.stream.Collectors; import javax.servlet.AsyncContext; import javax.servlet.DispatcherType; import javax.servlet.RequestDispatcher; @@ -58,6 +59,8 @@ import javax.servlet.http.Part; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; +import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.LinkedCaseInsensitiveMap; import org.springframework.util.LinkedMultiValueMap; @@ -168,10 +171,13 @@ public class MockHttpServletRequest implements HttpServletRequest { private final Map attributes = new LinkedHashMap<>(); + @Nullable private String characterEncoding; + @Nullable private byte[] content; + @Nullable private String contentType; private final Map parameters = new LinkedHashMap<>(16); @@ -205,6 +211,7 @@ public class MockHttpServletRequest implements HttpServletRequest { private boolean asyncSupported = false; + @Nullable private MockAsyncContext asyncContext; private DispatcherType dispatcherType = DispatcherType.REQUEST; @@ -214,32 +221,42 @@ public class MockHttpServletRequest implements HttpServletRequest { // HttpServletRequest properties // --------------------------------------------------------------------- + @Nullable private String authType; + @Nullable private Cookie[] cookies; private final Map headers = new LinkedCaseInsensitiveMap<>(); + @Nullable private String method; + @Nullable private String pathInfo; private String contextPath = ""; + @Nullable private String queryString; + @Nullable private String remoteUser; private final Set userRoles = new HashSet<>(); + @Nullable private Principal userPrincipal; + @Nullable private String requestedSessionId; + @Nullable private String requestURI; private String servletPath = ""; + @Nullable private HttpSession session; private boolean requestedSessionIdValid = true; @@ -273,7 +290,7 @@ public class MockHttpServletRequest implements HttpServletRequest { * @see #setRequestURI * @see #MockHttpServletRequest(ServletContext, String, String) */ - public MockHttpServletRequest(String method, String requestURI) { + public MockHttpServletRequest(@Nullable String method, @Nullable String requestURI) { this(null, method, requestURI); } @@ -283,7 +300,7 @@ public class MockHttpServletRequest implements HttpServletRequest { * (may be {@code null} to use a default {@link MockServletContext}) * @see #MockHttpServletRequest(ServletContext, String, String) */ - public MockHttpServletRequest(ServletContext servletContext) { + public MockHttpServletRequest(@Nullable ServletContext servletContext) { this(servletContext, "", ""); } @@ -300,7 +317,7 @@ public class MockHttpServletRequest implements HttpServletRequest { * @see #setPreferredLocales * @see MockServletContext */ - public MockHttpServletRequest(ServletContext servletContext, String method, String requestURI) { + public MockHttpServletRequest(@Nullable ServletContext servletContext, @Nullable String method, @Nullable String requestURI) { this.servletContext = (servletContext != null ? servletContext : new MockServletContext()); this.method = method; this.requestURI = requestURI; @@ -369,12 +386,13 @@ public class MockHttpServletRequest implements HttpServletRequest { } @Override + @Nullable public String getCharacterEncoding() { return this.characterEncoding; } @Override - public void setCharacterEncoding(String characterEncoding) { + public void setCharacterEncoding(@Nullable String characterEncoding) { this.characterEncoding = characterEncoding; updateContentTypeHeader(); } @@ -399,7 +417,7 @@ public class MockHttpServletRequest implements HttpServletRequest { * @see #getContentAsByteArray() * @see #getContentAsString() */ - public void setContent(byte[] content) { + public void setContent(@Nullable byte[] content) { this.content = content; } @@ -410,6 +428,7 @@ public class MockHttpServletRequest implements HttpServletRequest { * @see #setContent(byte[]) * @see #getContentAsString() */ + @Nullable public byte[] getContentAsByteArray() { return this.content; } @@ -425,6 +444,7 @@ public class MockHttpServletRequest implements HttpServletRequest { * @see #setCharacterEncoding(String) * @see #getContentAsByteArray() */ + @Nullable public String getContentAsString() throws IllegalStateException, UnsupportedEncodingException { Assert.state(this.characterEncoding != null, "Cannot get content as a String for a null character encoding. " + @@ -446,7 +466,7 @@ public class MockHttpServletRequest implements HttpServletRequest { return getContentLength(); } - public void setContentType(String contentType) { + public void setContentType(@Nullable String contentType) { this.contentType = contentType; if (contentType != null) { try { @@ -467,6 +487,7 @@ public class MockHttpServletRequest implements HttpServletRequest { } @Override + @Nullable public String getContentType() { return this.contentType; } @@ -507,8 +528,7 @@ public class MockHttpServletRequest implements HttpServletRequest { */ public void setParameters(Map params) { Assert.notNull(params, "Parameter map must not be null"); - for (String key : params.keySet()) { - Object value = params.get(key); + params.forEach((key, value) -> { if (value instanceof String) { setParameter(key, (String) value); } @@ -519,7 +539,7 @@ public class MockHttpServletRequest implements HttpServletRequest { throw new IllegalArgumentException( "Parameter map value must be single value " + " or array of type [" + String.class.getName() + "]"); } - } + }); } /** @@ -527,7 +547,7 @@ public class MockHttpServletRequest implements HttpServletRequest { *

If there are already one or more values registered for the given * parameter name, the given value will be added to the end of the list. */ - public void addParameter(String name, String value) { + public void addParameter(String name, @Nullable String value) { addParameter(name, new String[] {value}); } @@ -557,8 +577,7 @@ public class MockHttpServletRequest implements HttpServletRequest { */ public void addParameters(Map params) { Assert.notNull(params, "Parameter map must not be null"); - for (String key : params.keySet()) { - Object value = params.get(key); + params.forEach((key, value) -> { if (value instanceof String) { addParameter(key, (String) value); } @@ -569,7 +588,7 @@ public class MockHttpServletRequest implements HttpServletRequest { throw new IllegalArgumentException("Parameter map value must be single value " + " or array of type [" + String.class.getName() + "]"); } - } + }); } /** @@ -588,6 +607,7 @@ public class MockHttpServletRequest implements HttpServletRequest { } @Override + @Nullable public String getParameter(String name) { Assert.notNull(name, "Parameter name must not be null"); String[] arr = this.parameters.get(name); @@ -708,7 +728,7 @@ public class MockHttpServletRequest implements HttpServletRequest { } @Override - public void setAttribute(String name, Object value) { + public void setAttribute(String name, @Nullable Object value) { checkActive(); Assert.notNull(name, "Attribute name must not be null"); if (value != null) { @@ -872,7 +892,7 @@ public class MockHttpServletRequest implements HttpServletRequest { } @Override - public AsyncContext startAsync(ServletRequest request, ServletResponse response) { + public AsyncContext startAsync(ServletRequest request, @Nullable ServletResponse response) { Assert.state(this.asyncSupported, "Async not supported"); this.asyncStarted = true; this.asyncContext = new MockAsyncContext(request, response); @@ -897,11 +917,12 @@ public class MockHttpServletRequest implements HttpServletRequest { return this.asyncSupported; } - public void setAsyncContext(MockAsyncContext asyncContext) { + public void setAsyncContext(@Nullable MockAsyncContext asyncContext) { this.asyncContext = asyncContext; } @Override + @Nullable public AsyncContext getAsyncContext() { return this.asyncContext; } @@ -920,26 +941,34 @@ public class MockHttpServletRequest implements HttpServletRequest { // HttpServletRequest interface // --------------------------------------------------------------------- - public void setAuthType(String authType) { + public void setAuthType(@Nullable String authType) { this.authType = authType; } @Override + @Nullable public String getAuthType() { return this.authType; } - public void setCookies(Cookie... cookies) { + public void setCookies(@Nullable Cookie... cookies) { this.cookies = (ObjectUtils.isEmpty(cookies) ? null : cookies); - this.headers.remove(HttpHeaders.COOKIE); - if (this.cookies != null) { - Arrays.stream(this.cookies) - .map(c -> c.getName() + '=' + (c.getValue() == null ? "" : c.getValue())) - .forEach(value -> doAddHeaderValue(HttpHeaders.COOKIE, value, false)); + if (this.cookies == null) { + removeHeader(HttpHeaders.COOKIE); + } + else { + doAddHeaderValue(HttpHeaders.COOKIE, encodeCookies(this.cookies), true); } } + private static String encodeCookies(@NonNull Cookie... cookies) { + return Arrays.stream(cookies) + .map(c -> c.getName() + '=' + (c.getValue() == null ? "" : c.getValue())) + .collect(Collectors.joining("; ")); + } + @Override + @Nullable public Cookie[] getCookies() { return this.cookies; } @@ -983,7 +1012,7 @@ public class MockHttpServletRequest implements HttpServletRequest { } } - private void doAddHeaderValue(String name, Object value, boolean replace) { + private void doAddHeaderValue(String name, @Nullable Object value, boolean replace) { HeaderValueHolder header = HeaderValueHolder.getByName(this.headers, name); Assert.notNull(value, "Header value must not be null"); if (header == null || replace) { @@ -1059,6 +1088,7 @@ public class MockHttpServletRequest implements HttpServletRequest { } @Override + @Nullable public String getHeader(String name) { HeaderValueHolder header = HeaderValueHolder.getByName(this.headers, name); return (header != null ? header.getStringValue() : null); @@ -1093,25 +1123,28 @@ public class MockHttpServletRequest implements HttpServletRequest { } } - public void setMethod(String method) { + public void setMethod(@Nullable String method) { this.method = method; } @Override + @Nullable public String getMethod() { return this.method; } - public void setPathInfo(String pathInfo) { + public void setPathInfo(@Nullable String pathInfo) { this.pathInfo = pathInfo; } @Override + @Nullable public String getPathInfo() { return this.pathInfo; } @Override + @Nullable public String getPathTranslated() { return (this.pathInfo != null ? getRealPath(this.pathInfo) : null); } @@ -1125,20 +1158,22 @@ public class MockHttpServletRequest implements HttpServletRequest { return this.contextPath; } - public void setQueryString(String queryString) { + public void setQueryString(@Nullable String queryString) { this.queryString = queryString; } @Override + @Nullable public String getQueryString() { return this.queryString; } - public void setRemoteUser(String remoteUser) { + public void setRemoteUser(@Nullable String remoteUser) { this.remoteUser = remoteUser; } @Override + @Nullable public String getRemoteUser() { return this.remoteUser; } @@ -1153,29 +1188,32 @@ public class MockHttpServletRequest implements HttpServletRequest { ((MockServletContext) this.servletContext).getDeclaredRoles().contains(role))); } - public void setUserPrincipal(Principal userPrincipal) { + public void setUserPrincipal(@Nullable Principal userPrincipal) { this.userPrincipal = userPrincipal; } @Override + @Nullable public Principal getUserPrincipal() { return this.userPrincipal; } - public void setRequestedSessionId(String requestedSessionId) { + public void setRequestedSessionId(@Nullable String requestedSessionId) { this.requestedSessionId = requestedSessionId; } @Override + @Nullable public String getRequestedSessionId() { return this.requestedSessionId; } - public void setRequestURI(String requestURI) { + public void setRequestURI(@Nullable String requestURI) { this.requestURI = requestURI; } @Override + @Nullable public String getRequestURI() { return this.requestURI; } @@ -1216,6 +1254,7 @@ public class MockHttpServletRequest implements HttpServletRequest { } @Override + @Nullable public HttpSession getSession(boolean create) { checkActive(); // Reset session if invalidated. @@ -1230,6 +1269,7 @@ public class MockHttpServletRequest implements HttpServletRequest { } @Override + @Nullable public HttpSession getSession() { return getSession(true); } @@ -1240,6 +1280,7 @@ public class MockHttpServletRequest implements HttpServletRequest { * Otherwise it simply returns the current session id. * @since 4.0.3 */ + @Override public String changeSessionId() { Assert.isTrue(this.session != null, "The request does not have a session"); if (this.session instanceof MockHttpSession) { @@ -1303,6 +1344,7 @@ public class MockHttpServletRequest implements HttpServletRequest { } @Override + @Nullable public Part getPart(String name) throws IOException, ServletException { return this.parts.getFirst(name); }