Skip to content

Commit 92a8927

Browse files
committed
Fix unintentional conversion of + to during rule processing
Also silences exception logging when running tests or when using the RewriteValve in embedded mode and setting the configuration directly.
1 parent 13ecfa2 commit 92a8927

3 files changed

Lines changed: 23 additions & 5 deletions

File tree

java/org/apache/catalina/valves/rewrite/RewriteValve.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@
1717
package org.apache.catalina.valves.rewrite;
1818

1919
import java.io.BufferedReader;
20+
import java.io.FileNotFoundException;
2021
import java.io.IOException;
2122
import java.io.InputStream;
2223
import java.io.InputStreamReader;
2324
import java.io.StringReader;
24-
import java.net.URLDecoder;
2525
import java.nio.charset.Charset;
2626
import java.nio.charset.StandardCharsets;
2727
import java.util.ArrayList;
@@ -173,7 +173,8 @@ protected void startInternal() throws LifecycleException {
173173
String webInfResourcePath = "/WEB-INF/" + resourcePath;
174174
is = ((Context) getContainer()).getServletContext().getResourceAsStream(webInfResourcePath);
175175
if (is == null) {
176-
if (containerLog.isInfoEnabled()) {
176+
// Don't log if the rules have been set directly via setConfiguration()
177+
if (containerLog.isInfoEnabled() && rules == null) {
177178
containerLog.info(sm.getString("rewriteValve.noConfiguration", webInfResourcePath));
178179
}
179180
} else {
@@ -186,6 +187,13 @@ protected void startInternal() throws LifecycleException {
186187
try {
187188
ConfigurationSource.Resource resource = ConfigFileLoader.getSource().getResource(resourceName);
188189
is = resource.getInputStream();
190+
} catch (FileNotFoundException fnfe) {
191+
// Don't log if the rules have been set directly via setConfiguration()
192+
if (rules == null) {
193+
if (containerLog.isInfoEnabled()) {
194+
containerLog.info(sm.getString("rewriteValve.noConfiguration", resourceName), fnfe);
195+
}
196+
}
189197
} catch (IOException ioe) {
190198
if (containerLog.isInfoEnabled()) {
191199
containerLog.info(sm.getString("rewriteValve.noConfiguration", resourceName), ioe);
@@ -584,7 +592,7 @@ public void invoke(Request request, Response response) throws IOException, Servl
584592
chunk.append(REWRITE_DEFAULT_ENCODER.encode(urlStringRewriteEncoded, uriCharset));
585593
// Rewriting may have denormalized the URL and added encoded characters
586594
// Decode then normalize
587-
String urlStringRewriteDecoded = URLDecoder.decode(urlStringRewriteEncoded, uriCharset);
595+
String urlStringRewriteDecoded = UDecoder.URLDecode(urlStringRewriteEncoded, uriCharset);
588596
urlStringRewriteDecoded = RequestUtil.normalize(urlStringRewriteDecoded);
589597
if (urlStringRewriteDecoded == null) {
590598
// Assume bad input caused the re-write to try and escape root

test/org/apache/catalina/valves/rewrite/TestRewriteValve.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.io.IOException;
2121
import java.io.PrintWriter;
2222
import java.net.HttpURLConnection;
23-
import java.net.URLDecoder;
2423
import java.nio.charset.StandardCharsets;
2524
import java.util.Arrays;
2625
import java.util.HashMap;
@@ -45,6 +44,7 @@
4544
import org.apache.catalina.startup.TomcatBaseTest;
4645
import org.apache.catalina.valves.ValveBase;
4746
import org.apache.tomcat.util.buf.ByteChunk;
47+
import org.apache.tomcat.util.buf.UDecoder;
4848
import org.apache.tomcat.util.http.Method;
4949

5050
/*
@@ -1081,6 +1081,12 @@ public void testEncodedUriEncodedSemicolon03() throws Exception {
10811081
}
10821082

10831083

1084+
@Test
1085+
public void testEncodedUriPlus() throws Exception {
1086+
doTestRewriteWithEncoding("a+b");
1087+
}
1088+
1089+
10841090
private void doTestRewriteWithEncoding(String segment) throws Exception {
10851091
doTestRewriteWithEncoding(segment, segment, null);
10861092
}
@@ -1111,7 +1117,7 @@ private void doTestRewriteWithEncoding(String segment, String expectedSegment, S
11111117
String body = res.toString();
11121118
Assert.assertTrue(body, body.contains("REQUEST-URI: /target/" + expectedSegment));
11131119
Assert.assertTrue(body, body.contains("PATH-INFO: /" +
1114-
URLDecoder.decode(expectedSegment, StandardCharsets.UTF_8)));
1120+
UDecoder.URLDecode(expectedSegment, StandardCharsets.UTF_8)));
11151121
Assert.assertTrue(body, body.contains("REQUEST-QUERY-STRING: " + expectedQueryString));
11161122
}
11171123

webapps/docs/changelog.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,10 @@
260260
<fix>
261261
Various edge cases for SSI substitutions, quoting and escaping. (remm)
262262
</fix>
263+
<fix>
264+
Fix unintentional conversion of literal <code>+</code> to a space during
265+
rule processing in the <code>RewriteValve</code>. (markt)
266+
</fix>
263267
</changelog>
264268
</subsection>
265269
<subsection name="Coyote">

0 commit comments

Comments
 (0)