From 7a3ad4ab4151e672ffe3348d035ec44a31ee9c62 Mon Sep 17 00:00:00 2001 From: Bernardo Sauer <53480612+MajaChen@users.noreply.github.com> Date: Fri, 19 Jan 2024 11:19:35 +0800 Subject: [PATCH] Hotfix rest Chinese encoding (#13617) * decode uri in RequestFacade * supplement the unit test * fix code style violations * remove duplicate codes * consider Accept-Charset as enc * run mvn spotless apply * add comment * take weight into consideration * fix code style violations according sonar * add DEFAULT_CHARSET --------- Co-authored-by: chenxinyuan1 --- .../remoting/http/rest/RestClientTest.java | 47 +++++++++++++++++++ .../protocol/rest/constans/RestConstant.java | 3 ++ .../protocol/rest/request/RequestFacade.java | 23 +++++++++ .../protocol/rest/util/DataParseUtils.java | 37 +++++++++++++++ .../rpc/protocol/rest/DataParseUtilsTest.java | 11 +++++ .../protocol/rest/NettyRequestFacadeTest.java | 35 ++++++++++++++ 6 files changed, 156 insertions(+) diff --git a/dubbo-remoting/dubbo-remoting-http/src/test/java/org/apache/dubbo/remoting/http/rest/RestClientTest.java b/dubbo-remoting/dubbo-remoting-http/src/test/java/org/apache/dubbo/remoting/http/rest/RestClientTest.java index 3f1491e94b..571861ae3d 100644 --- a/dubbo-remoting/dubbo-remoting-http/src/test/java/org/apache/dubbo/remoting/http/rest/RestClientTest.java +++ b/dubbo-remoting/dubbo-remoting-http/src/test/java/org/apache/dubbo/remoting/http/rest/RestClientTest.java @@ -190,4 +190,51 @@ public class RestClientTest { strings.toArray(new String[0]), requestTemplate.getParam("param").toArray(new String[0])); } + + @Test + void testBuildURL() throws Exception { + int port = NetUtils.getAvailablePort(); + URL url = new ServiceConfigURL( + "http", "localhost", port, new String[] {Constants.BIND_PORT_KEY, String.valueOf(port)}); + HttpServer httpServer = new JettyHttpServer(url, new HttpHandler() { + @Override + public void handle(HttpServletRequest request, HttpServletResponse response) throws IOException { + response.setCharacterEncoding("UTF-8"); + response.getWriter().write(request.getQueryString()); + } + }); + + RequestTemplate requestTemplate = new RequestTemplate(null, "POST", "localhost:" + port); + + requestTemplate.addParam("name", "李强"); + requestTemplate.addParam("age", "18"); + requestTemplate.path("/hello/world"); + + // When using the OKHttpRestClient, parameters will be encoded with UTF-8 and appended to the URL + RestClient restClient = new OKHttpRestClient(new HttpClientConfig()); + + CompletableFuture send = restClient.send(requestTemplate); + + RestResult restResult = send.get(); + + assertThat(new String(restResult.getBody()), is("name=%E6%9D%8E%E5%BC%BA&age=18")); + + // When using the HttpClientRestClient, parameters will be encoded with UTF-8 and appended to the URL + restClient = new HttpClientRestClient(new HttpClientConfig()); + + send = restClient.send(requestTemplate); + + restResult = send.get(); + + assertThat(new String(restResult.getBody()), is("name=%E6%9D%8E%E5%BC%BA&age=18")); + + // When using the URLConnectionRestClient, parameters won't be encoded and still appended to the URL + restClient = new URLConnectionRestClient(new HttpClientConfig()); + + send = restClient.send(requestTemplate); + + restResult = send.get(); + + assertThat(new String(restResult.getBody(), StandardCharsets.UTF_8), is("name=李强&age=18")); + } } diff --git a/dubbo-rpc/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/constans/RestConstant.java b/dubbo-rpc/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/constans/RestConstant.java index e50f780525..a8d5bbc174 100644 --- a/dubbo-rpc/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/constans/RestConstant.java +++ b/dubbo-rpc/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/constans/RestConstant.java @@ -43,6 +43,8 @@ public interface RestConstant { String CONNECTION = "Connection"; String CONTENT_TYPE = "Content-Type"; String TEXT_PLAIN = "text/plain"; + String ACCEPT_CHARSET = "Accept-Charset"; + String WEIGHT_IDENTIFIER = ";q="; String ACCEPT = "Accept"; String DEFAULT_ACCEPT = "*/*"; String REST_HEADER_PREFIX = "rest-service-"; @@ -54,6 +56,7 @@ public interface RestConstant { String MAX_REQUEST_SIZE_PARAM = "max.request.size"; String IDLE_TIMEOUT_PARAM = "idle.timeout"; String KEEP_ALIVE_TIMEOUT_PARAM = "keep.alive.timeout"; + String DEFAULT_CHARSET = "UTF-8"; int MAX_REQUEST_SIZE = 1024 * 1024 * 10; int MAX_INITIAL_LINE_LENGTH = 4096; diff --git a/dubbo-rpc/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/request/RequestFacade.java b/dubbo-rpc/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/request/RequestFacade.java index e908a55239..c3c4ac5bb8 100644 --- a/dubbo-rpc/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/request/RequestFacade.java +++ b/dubbo-rpc/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/request/RequestFacade.java @@ -17,14 +17,19 @@ package org.apache.dubbo.rpc.protocol.rest.request; import org.apache.dubbo.common.utils.StringUtils; +import org.apache.dubbo.rpc.protocol.rest.constans.RestConstant; import org.apache.dubbo.rpc.protocol.rest.deploy.ServiceDeployer; +import org.apache.dubbo.rpc.protocol.rest.util.DataParseUtils; import java.io.IOException; +import java.net.URLDecoder; import java.util.ArrayList; import java.util.Enumeration; import java.util.HashMap; import java.util.Map; +import static org.apache.dubbo.rpc.protocol.rest.constans.RestConstant.DEFAULT_CHARSET; + /** * request facade for different request * @@ -55,6 +60,24 @@ public abstract class RequestFacade { protected void initParameters() { String requestURI = getRequestURI(); + String decodedRequestURI = null; + + try { + String enc = DEFAULT_CHARSET; + ArrayList charset = headers.get(RestConstant.ACCEPT_CHARSET); + // take the highest priority charset + String[] parsed = DataParseUtils.parseAcceptCharset(charset); + if (parsed != null && parsed.length > 0) { + enc = parsed[0].toUpperCase(); + } + decodedRequestURI = URLDecoder.decode(requestURI, enc); + } catch (Throwable t) { + // do nothing, try best to deliver + } + + if (StringUtils.isNotEmpty(decodedRequestURI)) { + requestURI = decodedRequestURI; + } if (requestURI != null && requestURI.contains("?")) { diff --git a/dubbo-rpc/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/util/DataParseUtils.java b/dubbo-rpc/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/util/DataParseUtils.java index ceeae1c745..489d99b905 100644 --- a/dubbo-rpc/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/util/DataParseUtils.java +++ b/dubbo-rpc/dubbo-rpc-rest/src/main/java/org/apache/dubbo/rpc/protocol/rest/util/DataParseUtils.java @@ -16,6 +16,8 @@ */ package org.apache.dubbo.rpc.protocol.rest.util; +import org.apache.dubbo.common.lang.Nullable; +import org.apache.dubbo.common.utils.CollectionUtils; import org.apache.dubbo.common.utils.JsonUtils; import org.apache.dubbo.common.utils.StringUtils; @@ -29,9 +31,16 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collection; +import java.util.Comparator; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.SortedMap; import java.util.StringTokenizer; +import java.util.TreeMap; + +import static org.apache.dubbo.rpc.protocol.rest.constans.RestConstant.WEIGHT_IDENTIFIER; public class DataParseUtils { @@ -209,4 +218,32 @@ public class DataParseUtils { public static String[] toStringArray(Collection collection) { return collection == null ? null : collection.toArray(new String[collection.size()]); } + + @Nullable + public static String[] parseAcceptCharset(List acceptCharsets) { + if (CollectionUtils.isEmpty(acceptCharsets)) { + return new String[0]; + } + + SortedMap> encodings = new TreeMap<>(Comparator.reverseOrder()); + float defaultWeight = 1.0f; + for (String acceptCharset : acceptCharsets) { + String[] charsets = acceptCharset.split(","); + for (String charset : charsets) { + charset = charset.trim(); + float weight = defaultWeight; + String enc = charset; + if (charset.contains(WEIGHT_IDENTIFIER)) { + String[] split = charset.split(WEIGHT_IDENTIFIER); + enc = split[0]; + weight = Float.parseFloat(split[1]); + } + encodings.computeIfAbsent(weight, k -> new HashSet<>()).add(enc); + } + } + + List result = new ArrayList<>(); + encodings.values().forEach(result::addAll); + return result.toArray(new String[0]); + } } diff --git a/dubbo-rpc/dubbo-rpc-rest/src/test/java/org/apache/dubbo/rpc/protocol/rest/DataParseUtilsTest.java b/dubbo-rpc/dubbo-rpc-rest/src/test/java/org/apache/dubbo/rpc/protocol/rest/DataParseUtilsTest.java index 5b3898eb26..896a402815 100644 --- a/dubbo-rpc/dubbo-rpc-rest/src/test/java/org/apache/dubbo/rpc/protocol/rest/DataParseUtilsTest.java +++ b/dubbo-rpc/dubbo-rpc-rest/src/test/java/org/apache/dubbo/rpc/protocol/rest/DataParseUtilsTest.java @@ -19,6 +19,7 @@ package org.apache.dubbo.rpc.protocol.rest; import org.apache.dubbo.rpc.protocol.rest.util.DataParseUtils; import java.io.ByteArrayOutputStream; +import java.util.Arrays; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -56,4 +57,14 @@ public class DataParseUtilsTest { Assertions.assertEquals(1, convert); } + + @Test + void testParseAcceptCharset() { + String[] parsed = DataParseUtils.parseAcceptCharset(Arrays.asList("iso-8859-1")); + Assertions.assertTrue(Arrays.equals(parsed, new String[] {"iso-8859-1"})); + parsed = DataParseUtils.parseAcceptCharset(Arrays.asList("utf-8, iso-8859-1;q=0.5")); + Assertions.assertTrue(Arrays.equals(parsed, new String[] {"utf-8", "iso-8859-1"})); + parsed = DataParseUtils.parseAcceptCharset(Arrays.asList("utf-8, iso-8859-1;q=0.5, *;q=0.1", "utf-16;q=0.5")); + Assertions.assertEquals("utf-8", parsed[0]); + } } diff --git a/dubbo-rpc/dubbo-rpc-rest/src/test/java/org/apache/dubbo/rpc/protocol/rest/NettyRequestFacadeTest.java b/dubbo-rpc/dubbo-rpc-rest/src/test/java/org/apache/dubbo/rpc/protocol/rest/NettyRequestFacadeTest.java index 87a30ad32c..c923fb1d8b 100644 --- a/dubbo-rpc/dubbo-rpc-rest/src/test/java/org/apache/dubbo/rpc/protocol/rest/NettyRequestFacadeTest.java +++ b/dubbo-rpc/dubbo-rpc-rest/src/test/java/org/apache/dubbo/rpc/protocol/rest/NettyRequestFacadeTest.java @@ -30,6 +30,9 @@ import io.netty.handler.codec.http.HttpVersion; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.Is.is; + public class NettyRequestFacadeTest { @Test @@ -95,4 +98,36 @@ public class NettyRequestFacadeTest { Assertions.assertEquals("GET", nettyRequestFacade.getMethod()); } + + @Test + void testChineseDecoding() { + String uri = "/hello/world?name=%E6%9D%8E%E5%BC%BA&age=18"; + DefaultFullHttpRequest defaultFullHttpRequest = + new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, uri); + defaultFullHttpRequest.headers().add("Accept-Charset", "utf-8, iso-8859-1;q=0.5, *;q=0.1"); + defaultFullHttpRequest.headers().add("Accept-Charset", "utf-16;q=0.3"); + + NettyRequestFacade nettyRequestFacade = new NettyRequestFacade(defaultFullHttpRequest, null); + assertThat(nettyRequestFacade.getPath(), is("/hello/world")); + assertThat(nettyRequestFacade.getParameter("name"), is("李强")); + assertThat(nettyRequestFacade.getParameter("age"), is("18")); + + // Applying the decode method to the URI is acceptable, even if the URI is not encoded. + uri = "/hello/world?name=lily&age=18"; + defaultFullHttpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, uri); + + nettyRequestFacade = new NettyRequestFacade(defaultFullHttpRequest, null); + assertThat(nettyRequestFacade.getPath(), is("/hello/world")); + assertThat(nettyRequestFacade.getParameter("name"), is("lily")); + assertThat(nettyRequestFacade.getParameter("age"), is("18")); + + // When using URLConnectionRestClient, the URI won't be encoded, but it's still acceptable. + uri = "/hello/world?name=李强&age=18"; + defaultFullHttpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, uri); + + nettyRequestFacade = new NettyRequestFacade(defaultFullHttpRequest, null); + assertThat(nettyRequestFacade.getPath(), is("/hello/world")); + assertThat(nettyRequestFacade.getParameter("name"), is("李强")); + assertThat(nettyRequestFacade.getParameter("age"), is("18")); + } }