From 0d1b7fd14f495bdbbaf8c5d05f8bfb884b133a47 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 7 Jul 2015 00:47:06 -0400 Subject: [PATCH] Fix RequestPartServletServerHttpRequest encoding issue When using Appache Commons FileUpload, multi parts with binary data (i.e. that are not actual files) are saved and then accessed as String request parameters. Before this change however the RequestPartServletServerHttpRequest used a fixed encoding (UTF-8) while the parsing code in CommonsFileUploadSupport/Resolver used the encoding from the content-type header, or the request, or the FileUpload component. This change does a best effort to determine the encoding of the request parameter using a similar algorithm as the parsing side that should work the same unless the encoding comes from the FileUpload component which is not accessible. Issue: SPR-13096 --- .../RequestPartServletServerHttpRequest.java | 18 +++- ...uestPartServletServerHttpRequestTests.java | 101 +++++++++++++----- .../RequestPartIntegrationTests.java | 12 ++- 3 files changed, 101 insertions(+), 30 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/multipart/support/RequestPartServletServerHttpRequest.java b/spring-web/src/main/java/org/springframework/web/multipart/support/RequestPartServletServerHttpRequest.java index 152f21ef52..7e89d7c08f 100644 --- a/spring-web/src/main/java/org/springframework/web/multipart/support/RequestPartServletServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/web/multipart/support/RequestPartServletServerHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2015 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. @@ -19,9 +19,11 @@ package org.springframework.web.multipart.support; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.charset.Charset; import javax.servlet.http.HttpServletRequest; import org.springframework.http.HttpHeaders; +import org.springframework.http.MediaType; import org.springframework.http.server.ServerHttpRequest; import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.util.ClassUtils; @@ -111,9 +113,21 @@ public class RequestPartServletServerHttpRequest extends ServletServerHttpReques } else { String paramValue = this.multipartRequest.getParameter(this.partName); - return new ByteArrayInputStream(paramValue.getBytes(FORM_CHARSET)); + return new ByteArrayInputStream(paramValue.getBytes(determineEncoding())); } } } + private String determineEncoding() { + MediaType contentType = getHeaders().getContentType(); + if (contentType != null) { + Charset charset = contentType.getCharSet(); + if (charset != null) { + return charset.name(); + } + } + String encoding = this.multipartRequest.getCharacterEncoding(); + return (encoding != null ? encoding : FORM_CHARSET); + } + } diff --git a/spring-web/src/test/java/org/springframework/web/multipart/support/RequestPartServletServerHttpRequestTests.java b/spring-web/src/test/java/org/springframework/web/multipart/support/RequestPartServletServerHttpRequestTests.java index 60776274c5..df611a2b19 100644 --- a/spring-web/src/test/java/org/springframework/web/multipart/support/RequestPartServletServerHttpRequestTests.java +++ b/spring-web/src/test/java/org/springframework/web/multipart/support/RequestPartServletServerHttpRequestTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2015 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. @@ -17,6 +17,7 @@ package org.springframework.web.multipart.support; import java.net.URI; +import java.nio.charset.Charset; import org.junit.Before; import org.junit.Test; @@ -24,61 +25,107 @@ import org.junit.Test; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; +import org.springframework.http.server.ServerHttpRequest; import org.springframework.mock.web.test.MockMultipartFile; import org.springframework.mock.web.test.MockMultipartHttpServletRequest; import org.springframework.util.FileCopyUtils; +import org.springframework.web.multipart.MultipartFile; -import static org.junit.Assert.*; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; /** * @author Rossen Stoyanchev */ public class RequestPartServletServerHttpRequestTests { - private RequestPartServletServerHttpRequest request; + private final MockMultipartHttpServletRequest mockRequest = new MockMultipartHttpServletRequest(); - private MockMultipartHttpServletRequest mockRequest; - - private MockMultipartFile mockFile; - - @Before - public void create() throws Exception { - mockFile = new MockMultipartFile("part", "", "application/json" ,"Part Content".getBytes("UTF-8")); - mockRequest = new MockMultipartHttpServletRequest(); - mockRequest.addFile(mockFile); - request = new RequestPartServletServerHttpRequest(mockRequest, "part"); - } @Test public void getMethod() throws Exception { - mockRequest.setMethod("POST"); - assertEquals("Invalid method", HttpMethod.POST, request.getMethod()); + this.mockRequest.addFile(new MockMultipartFile("part", "", "", "content".getBytes("UTF-8"))); + ServerHttpRequest request = new RequestPartServletServerHttpRequest(this.mockRequest, "part"); + this.mockRequest.setMethod("POST"); + + assertEquals(HttpMethod.POST, request.getMethod()); } @Test public void getURI() throws Exception { + this.mockRequest.addFile(new MockMultipartFile("part", "", "application/json", "content".getBytes("UTF-8"))); + ServerHttpRequest request = new RequestPartServletServerHttpRequest(this.mockRequest, "part"); + URI uri = new URI("http://example.com/path?query"); - mockRequest.setServerName(uri.getHost()); - mockRequest.setServerPort(uri.getPort()); - mockRequest.setRequestURI(uri.getPath()); - mockRequest.setQueryString(uri.getQuery()); - assertEquals("Invalid uri", uri, request.getURI()); + this.mockRequest.setServerName(uri.getHost()); + this.mockRequest.setServerPort(uri.getPort()); + this.mockRequest.setRequestURI(uri.getPath()); + this.mockRequest.setQueryString(uri.getQuery()); + assertEquals(uri, request.getURI()); } @Test public void getContentType() throws Exception { - HttpHeaders headers = request.getHeaders(); - assertNotNull("No HttpHeaders returned", headers); + MultipartFile part = new MockMultipartFile("part", "", "application/json", "content".getBytes("UTF-8")); + this.mockRequest.addFile(part); + ServerHttpRequest request = new RequestPartServletServerHttpRequest(this.mockRequest, "part"); - MediaType expected = MediaType.parseMediaType(mockFile.getContentType()); - MediaType actual = headers.getContentType(); - assertEquals("Invalid content type returned", expected, actual); + HttpHeaders headers = request.getHeaders(); + assertNotNull(headers); + assertEquals(MediaType.APPLICATION_JSON, headers.getContentType()); } @Test public void getBody() throws Exception { + byte[] bytes = "content".getBytes("UTF-8"); + MultipartFile part = new MockMultipartFile("part", "", "application/json", bytes); + this.mockRequest.addFile(part); + ServerHttpRequest request = new RequestPartServletServerHttpRequest(this.mockRequest, "part"); + byte[] result = FileCopyUtils.copyToByteArray(request.getBody()); - assertArrayEquals("Invalid content returned", mockFile.getBytes(), result); + assertArrayEquals(bytes, result); + } + + // SPR-13096 + + @Test + public void getBodyViaRequestParameter() throws Exception { + MockMultipartHttpServletRequest mockRequest = new MockMultipartHttpServletRequest() { + + @Override + public HttpHeaders getMultipartHeaders(String paramOrFileName) { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(new MediaType("application", "octet-stream", Charset.forName("iso-8859-1"))); + return headers; + } + }; + byte[] bytes = {(byte) 0xC4}; + mockRequest.setParameter("part", new String(bytes, Charset.forName("iso-8859-1"))); + ServerHttpRequest request = new RequestPartServletServerHttpRequest(mockRequest, "part"); + + byte[] result = FileCopyUtils.copyToByteArray(request.getBody()); + assertArrayEquals(bytes, result); + } + + @Test + public void getBodyViaRequestParameterWithRequestEncoding() throws Exception { + MockMultipartHttpServletRequest mockRequest = new MockMultipartHttpServletRequest() { + + @Override + public HttpHeaders getMultipartHeaders(String paramOrFileName) { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(MediaType.APPLICATION_OCTET_STREAM); + return headers; + } + }; + byte[] bytes = {(byte) 0xC4}; + mockRequest.setParameter("part", new String(bytes, Charset.forName("iso-8859-1"))); + mockRequest.setCharacterEncoding("iso-8859-1"); + ServerHttpRequest request = new RequestPartServletServerHttpRequest(mockRequest, "part"); + + byte[] result = FileCopyUtils.copyToByteArray(request.getBody()); + assertArrayEquals(bytes, result); } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartIntegrationTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartIntegrationTests.java index 67ffce7aca..812629b48c 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartIntegrationTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartIntegrationTests.java @@ -19,6 +19,7 @@ package org.springframework.web.servlet.mvc.method.annotation; import static org.junit.Assert.*; import java.net.URI; +import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -29,6 +30,7 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.junit.AfterClass; +import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -112,6 +114,7 @@ public class RequestPartIntegrationTests { List> converters = new ArrayList<>(3); converters.add(emptyBodyConverter); + converters.add(new ByteArrayHttpMessageConverter()); converters.add(new ResourceHttpMessageConverter()); converters.add(new MappingJackson2HttpMessageConverter()); @@ -146,6 +149,10 @@ public class RequestPartIntegrationTests { parts.add("file-data", new ClassPathResource("logo.jpg", this.getClass())); parts.add("empty-data", new HttpEntity(new byte[0])); // SPR-12860 + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(new MediaType("application", "octet-stream", Charset.forName("ISO-8859-1"))); + parts.add("iso-8859-1-data", new HttpEntity(new byte[] {(byte) 0xC4}, headers)); // SPR-13096 + URI location = restTemplate.postForLocation(url, parts); assertEquals("http://localhost:8080/test/Jason/logo.jpg", location.toString()); } @@ -185,7 +192,10 @@ public class RequestPartIntegrationTests { @RequestMapping(value = "/test", method = RequestMethod.POST, consumes = { "multipart/mixed", "multipart/form-data" }) public ResponseEntity create(@RequestPart(name = "json-data") TestData testData, @RequestPart("file-data") MultipartFile file, - @RequestPart(name = "empty-data", required = false) TestData emptyData) { + @RequestPart(name = "empty-data", required = false) TestData emptyData, + @RequestPart(name = "iso-8859-1-data") byte[] iso88591Data) { + + Assert.assertArrayEquals(new byte[]{(byte) 0xC4}, iso88591Data); String url = "http://localhost:8080/test/" + testData.getName() + "/" + file.getOriginalFilename(); HttpHeaders headers = new HttpHeaders();