Skip to content

Commit

Permalink
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=58836
Browse files Browse the repository at this point in the history 
Correctly merge query string parameters when processing a forwarded request where the target includes a query string that contains a parameter with no value.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1724427 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@markt-asf
markt-asf committed Jan 13, 2016
1 parent 2cc3c1a commit 6df78f3
Show file tree
Hide file tree
Showing 3 changed files with 246 additions and 16 deletions.
40 changes: 24 additions & 16 deletions java/org/apache/catalina/core/ApplicationHttpRequest.java
View file
Expand Up @@ -25,7 +25,6 @@
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.NoSuchElementException;

import javax.servlet.DispatcherType;
Expand All @@ -40,7 +39,8 @@
import org.apache.catalina.Globals;
import org.apache.catalina.Manager;
import org.apache.catalina.Session;
import org.apache.catalina.util.RequestUtil;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.http.Parameters;


/**
Expand Down Expand Up @@ -885,24 +885,32 @@ private void mergeParameters() {
return;

HashMap<String, String[]> queryParameters = new HashMap<>();
String encoding = getCharacterEncoding();
if (encoding == null)
encoding = "ISO-8859-1";
RequestUtil.parseParameters(queryParameters, queryParamString,
encoding);
for (Entry<String, String[]> entry : parameters.entrySet()) {
String entryKey = entry.getKey();
String[] entryValue = entry.getValue();
Object value = queryParameters.get(entryKey);
if (value == null) {
queryParameters.put(entryKey, entryValue);

// Parse the query string from the dispatch target
Parameters paramParser = new Parameters();
MessageBytes queryMB = MessageBytes.newInstance();
queryMB.setString(queryParamString);
paramParser.setQuery(queryMB);
paramParser.setQueryStringEncoding(getCharacterEncoding());
paramParser.handleQueryParameters();

// Copy the original parameters
queryParameters.putAll(parameters);

// Insert the additional parameters from the dispatch target
Enumeration<String> dispParamNames = paramParser.getParameterNames();
while (dispParamNames.hasMoreElements()) {
String dispParamName = dispParamNames.nextElement();
String[] dispParamValues = paramParser.getParameterValues(dispParamName);
String[] originalValues = queryParameters.get(dispParamName);
if (originalValues == null) {
queryParameters.put(dispParamName, dispParamValues);
continue;
}
queryParameters.put
(entryKey, mergeValues(value, entryValue));
queryParameters.put(dispParamName, mergeValues(dispParamValues, originalValues));
}
parameters = queryParameters;

parameters = queryParameters;
}


Expand Down
217 changes: 217 additions & 0 deletions test/org/apache/catalina/core/TestApplicationHttpRequest.java
View file
@@ -0,0 +1,217 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.catalina.core;

import java.io.IOException;
import java.io.PrintWriter;
import java.util.Map;
import java.util.Map.Entry;

import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.junit.Assert;
import org.junit.Test;

import org.apache.catalina.Context;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
import org.apache.tomcat.util.buf.ByteChunk;

public class TestApplicationHttpRequest extends TomcatBaseTest {

/*
* https://bz.apache.org/bugzilla/show_bug.cgi?id=58836
*/
@Test
public void testForwardQueryString01() throws Exception {
doQueryStringTest(null, "a=b", "a:(b)");
}


@Test
public void testForwardQueryString02() throws Exception {
doQueryStringTest(null, "a=b&a=c", "a:(b),(c)");
}


@Test
public void testForwardQueryString03() throws Exception {
doQueryStringTest(null, "a=b&c=d", "a:(b);c:(d)");
}


@Test
public void testForwardQueryString04() throws Exception {
doQueryStringTest(null, "a=b&c=d&a=e", "a:(b),(e);c:(d)");
}


@Test
public void testForwardQueryString05() throws Exception {
// Parameters with no value are assigned a vale of the empty string
doQueryStringTest(null, "a=b&c&a=e", "a:(b),(e);c:()");
}


@Test
public void testOriginalQueryString01() throws Exception {
doQueryStringTest("a=b", null, "a:(b)");
}


@Test
public void testOriginalQueryString02() throws Exception {
doQueryStringTest("a=b&a=c", null, "a:(b),(c)");
}


@Test
public void testOriginalQueryString03() throws Exception {
doQueryStringTest("a=b&c=d", null, "a:(b);c:(d)");
}


@Test
public void testOriginalQueryString04() throws Exception {
doQueryStringTest("a=b&c=d&a=e", null, "a:(b),(e);c:(d)");
}


@Test
public void testOriginalQueryString05() throws Exception {
// Parameters with no value are assigned a vale of the empty string
doQueryStringTest("a=b&c&a=e", null, "a:(b),(e);c:()");
}


@Test
public void testMergeQueryString01() throws Exception {
doQueryStringTest("a=b", "a=z", "a:(z),(b)");
}


@Test
public void testMergeQueryString02() throws Exception {
// Parameters with no value are assigned a vale of the empty string
doQueryStringTest("a=b&c&a=e", "a=z", "a:(z),(b),(e);c:()");
}


@Test
public void testMergeQueryString03() throws Exception {
// Parameters with no value are assigned a vale of the empty string
doQueryStringTest("a=b&c&a=e", "c=z", "a:(b),(e);c:(z),()");
}


@Test
public void testMergeQueryString04() throws Exception {
// Parameters with no value are assigned a vale of the empty string
doQueryStringTest("a=b&c&a=e", "a", "a:(),(b),(e);c:()");
}


private void doQueryStringTest(String originalQueryString, String forwardQueryString,
String expected) throws Exception {
Tomcat tomcat = getTomcatInstance();

// No file system docBase required
Context ctx = tomcat.addContext("", null);

if (forwardQueryString == null) {
Tomcat.addServlet(ctx, "forward", new ForwardServlet("/display"));
} else {
Tomcat.addServlet(ctx, "forward", new ForwardServlet("/display?" + forwardQueryString));
}
ctx.addServletMapping("/forward", "forward");

Tomcat.addServlet(ctx, "display", new DisplayParameterServlet());
ctx.addServletMapping("/display", "display");

tomcat.start();

ByteChunk response = new ByteChunk();
StringBuilder target = new StringBuilder("http://localhost:");
target.append(getPort());
target.append("/forward");
if (originalQueryString != null) {
target.append('?');
target.append(originalQueryString);
}
int rc = getUrl(target.toString(), response, null);

Assert.assertEquals(200, rc);
Assert.assertEquals(expected, response.toString());
}


private static class ForwardServlet extends HttpServlet {

private static final long serialVersionUID = 1L;

private final String target;

public ForwardServlet(String target) {
this.target = target;
}

@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
req.getRequestDispatcher(target).forward(req, resp);
}
}


private static class DisplayParameterServlet extends HttpServlet {

private static final long serialVersionUID = 1L;

@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
resp.setContentType("text/plain");
resp.setCharacterEncoding("UTF-8");
PrintWriter w = resp.getWriter();
Map<String,String[]> params = req.getParameterMap();
boolean firstParam = true;
for (Entry<String,String[]> param : params.entrySet()) {
if (firstParam) {
firstParam = false;
} else {
w.print(';');
}
w.print(param.getKey());
w.print(':');
boolean firstValue = true;
for (String value : param.getValue()) {
if (firstValue) {
firstValue = false;
} else {
w.print(',');
}
w.print('(');
w.print(value);
w.print(')');
}
}
}
}
}
5 changes: 5 additions & 0 deletions webapps/docs/changelog.xml
View file
Expand Up @@ -179,6 +179,11 @@
Fix declaration of <code>localPort</code> attribute of Connector MBean:
it is read-only. (kkolinko)
</fix>
<fix>
<bug>58836</bug>: Correctly merge query string parameters when
processing a forwarded request where the target includes a query string
that contains a parameter with no value. (mark)
</fix>
</changelog>
</subsection>
<subsection name="Coyote">
Expand Down

0 comments on commit 6df78f3

Please sign in to comment.