From 5529843f0b9743b0c013784ffdc49e0c788597f5 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Mon, 21 Dec 2015 16:01:01 +0900 Subject: [PATCH] Fix missing query string conversion in TomcatService Motivation: TomcatServiceInvocationHandler does not set the 'queryString' property of Coyote Request object during the conversion. Modifications: - Call Request.queryString().setString() if HttpRequest.uri() has a query string - Add the test cases for GET and POST with query parameters Result: Web application gets query parameters correctly. --- .../TomcatServiceInvocationHandler.java | 16 +++++- .../server/http/tomcat/TomcatServiceTest.java | 49 ++++++++++++++++++- .../resources/tomcat_service/query_string.jsp | 7 +++ 3 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 src/test/resources/tomcat_service/query_string.jsp diff --git a/src/main/java/com/linecorp/armeria/server/http/tomcat/TomcatServiceInvocationHandler.java b/src/main/java/com/linecorp/armeria/server/http/tomcat/TomcatServiceInvocationHandler.java index 689d65af9c8..b1d28a272dd 100644 --- a/src/main/java/com/linecorp/armeria/server/http/tomcat/TomcatServiceInvocationHandler.java +++ b/src/main/java/com/linecorp/armeria/server/http/tomcat/TomcatServiceInvocationHandler.java @@ -57,6 +57,7 @@ import io.netty.handler.codec.http.FullHttpResponse; import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpHeaders; +import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpVersion; import io.netty.util.AsciiString; @@ -248,14 +249,27 @@ public long getBytesWritten() { private static Request convertRequest(FullHttpRequest req, String mappedPath) { final Request coyoteReq = new Request(); - coyoteReq.method().setString(req.method().name()); + + // Set the method. + final HttpMethod method = req.method(); + coyoteReq.method().setString(method.name()); + + // Set the request URI. final byte[] uriBytes = mappedPath.getBytes(StandardCharsets.US_ASCII); coyoteReq.requestURI().setBytes(uriBytes, 0, uriBytes.length); + // Set the query string if any. + final int queryIndex = req.uri().indexOf('?'); + if (queryIndex >= 0) { + coyoteReq.queryString().setString(req.uri().substring(queryIndex + 1)); + } + + // Set the headers. final MimeHeaders cHeaders = coyoteReq.getMimeHeaders(); convertHeaders(req.headers(), cHeaders); convertHeaders(req.trailingHeaders(), cHeaders); + // Set the content. final ByteBuf content = req.content(); if (content.isReadable()) { coyoteReq.setInputBuffer(new InputBuffer() { diff --git a/src/test/java/com/linecorp/armeria/server/http/tomcat/TomcatServiceTest.java b/src/test/java/com/linecorp/armeria/server/http/tomcat/TomcatServiceTest.java index c3d17872a13..b4c8d9c24e6 100644 --- a/src/test/java/com/linecorp/armeria/server/http/tomcat/TomcatServiceTest.java +++ b/src/test/java/com/linecorp/armeria/server/http/tomcat/TomcatServiceTest.java @@ -19,18 +19,23 @@ import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertThat; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Collections; import java.util.regex.Pattern; +import org.apache.http.client.entity.UrlEncodedFormEntity; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; +import org.apache.http.message.BasicNameValuePair; import org.apache.http.util.EntityUtils; import org.junit.Test; import com.linecorp.armeria.server.AbstractServerTest; import com.linecorp.armeria.server.ServerBuilder; -import com.linecorp.armeria.server.VirtualHostBuilder; import com.linecorp.armeria.server.logging.LoggingService; import io.netty.handler.codec.http.HttpHeaderNames; @@ -65,4 +70,46 @@ public void testJsp() throws Exception { } } } + + @Test + public void testGetQueryString() throws Exception { + try (CloseableHttpClient hc = HttpClients.createMinimal()) { + try (CloseableHttpResponse res = hc.execute( + new HttpGet(uri("/tc/query_string.jsp?foo=%31&bar=%32")))) { + + assertThat(res.getStatusLine().toString(), is("HTTP/1.1 200 OK")); + assertThat(res.getFirstHeader(HttpHeaderNames.CONTENT_TYPE.toString()).getValue(), + startsWith("text/html")); + final String actualContent = CR_OR_LF.matcher(EntityUtils.toString(res.getEntity())) + .replaceAll(""); + assertThat(actualContent, is( + "" + + "

foo is 1

" + + "

bar is 2

" + + "")); + } + } + } + + @Test + public void testPostQueryString() throws Exception { + try (CloseableHttpClient hc = HttpClients.createMinimal()) { + final HttpPost post = new HttpPost(uri("/tc/query_string.jsp?foo=3")); + post.setEntity(new UrlEncodedFormEntity( + Collections.singletonList(new BasicNameValuePair("bar", "4")), StandardCharsets.UTF_8)); + + try (CloseableHttpResponse res = hc.execute(post)) { + assertThat(res.getStatusLine().toString(), is("HTTP/1.1 200 OK")); + assertThat(res.getFirstHeader(HttpHeaderNames.CONTENT_TYPE.toString()).getValue(), + startsWith("text/html")); + final String actualContent = CR_OR_LF.matcher(EntityUtils.toString(res.getEntity())) + .replaceAll(""); + assertThat(actualContent, is( + "" + + "

foo is 3

" + + "

bar is 4

" + + "")); + } + } + } } diff --git a/src/test/resources/tomcat_service/query_string.jsp b/src/test/resources/tomcat_service/query_string.jsp new file mode 100644 index 00000000000..2e7886f7cb0 --- /dev/null +++ b/src/test/resources/tomcat_service/query_string.jsp @@ -0,0 +1,7 @@ +<%@ page contentType="text/html; ISO-8859-1" %> + + +

foo is <%= request.getParameter("foo") %>

+

bar is <%= request.getParameter("bar") %>

+ +