Merge pull request #74 from trustin/tomcat_query_string

Fix missing query string conversion in TomcatService
line · Dec 21, 2015 · 85970bd · 85970bd
2 parents 31b5f49 + 5529843
commit 85970bd
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 2 deletions.
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
@@ -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
@@ -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(
+                        "<html><body>" +
+                        "<p>foo is 1</p>" +
+                        "<p>bar is 2</p>" +
+                        "</body></html>"));
+            }
+        }
+    }
+
+    @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(
+                        "<html><body>" +
+                        "<p>foo is 3</p>" +
+                        "<p>bar is 4</p>" +
+                        "</body></html>"));
+            }
+        }
+    }
 }
diff --git a/src/test/resources/tomcat_service/query_string.jsp b/src/test/resources/tomcat_service/query_string.jsp
@@ -0,0 +1,7 @@
+<%@ page contentType="text/html; ISO-8859-1" %>
+<html>
+<body>
+<p>foo is <%= request.getParameter("foo") %></p>
+<p>bar is <%= request.getParameter("bar") %></p>
+</body>
+</html>