Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

only process percent encoded values on path as per RFC3986 #443

Merged
merged 1 commit into from
Sep 4, 2016
Merged

only process percent encoded values on path as per RFC3986 #443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions vertx-web/src/main/java/io/vertx/ext/web/handler/sockjs/impl/JsonPTransport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

package io.vertx.ext.web.handler.sockjs.impl;

import io.netty.handler.codec.http.QueryStringDecoder;
import io.vertx.core.Handler;
import io.vertx.core.Vertx;
import io.vertx.core.http.HttpServerRequest;
Expand All @@ -43,9 +44,6 @@
import io.vertx.ext.web.handler.sockjs.SockJSHandlerOptions;
import io.vertx.ext.web.handler.sockjs.SockJSSocket;

import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;

/**
* @author <a href="http://tfox.org">Tim Fox</a>
*/
Expand Down Expand Up @@ -116,12 +114,7 @@ private void handleSend(RoutingContext rc, SockJSSession session) {
}

if (urlEncoded) {
try {
body = URLDecoder.decode(body, "UTF-8");
} catch (UnsupportedEncodingException e) {
throw new IllegalStateException("No UTF-8!");
}
body = body.substring(2);
body = QueryStringDecoder.decodeComponent(body).substring(2);
}

if (!session.handleMessages(body)) {
Expand Down
45 changes: 17 additions & 28 deletions vertx-web/src/main/java/io/vertx/ext/web/impl/RouteImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package io.vertx.ext.web.impl;

import io.netty.handler.codec.http.QueryStringDecoder;
import io.vertx.core.Handler;
import io.vertx.core.http.HttpMethod;
import io.vertx.core.http.HttpServerRequest;
Expand All @@ -24,8 +25,6 @@
import io.vertx.ext.web.Route;
import io.vertx.ext.web.RoutingContext;

import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.util.*;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -251,39 +250,29 @@ synchronized boolean matches(RoutingContext context, String mountPoint, boolean
if (groups != null) {
// Pattern - named params
// decode the path as it could contain escaped chars.
try {
for (int i = 0; i < groups.size(); i++) {
final String k = groups.get(i);
final String value = URLDecoder.decode(m.group("p" + i), "UTF-8");
if (!request.params().contains(k)) {
params.put(k, value);
} else {
context.pathParams().put(k, value);
}
for (int i = 0; i < groups.size(); i++) {
final String k = groups.get(i);
final String value = QueryStringDecoder.decodeComponent(m.group("p" + i).replace("+", "%2b"));
if (!request.params().contains(k)) {
params.put(k, value);
} else {
context.pathParams().put(k, value);
}
} catch (UnsupportedEncodingException e) {
context.fail(e);
return false;
}
} else {
// Straight regex - un-named params
// decode the path as it could contain escaped chars.
try {
for (int i = 0; i < m.groupCount(); i++) {
String group = m.group(i + 1);
if(group != null) {
final String k = "param" + i;
final String value = URLDecoder.decode(group, "UTF-8");
if (!request.params().contains(k)) {
params.put(k, value);
} else {
context.pathParams().put(k, value);
}
for (int i = 0; i < m.groupCount(); i++) {
String group = m.group(i + 1);
if(group != null) {
final String k = "param" + i;
final String value = QueryStringDecoder.decodeComponent(group.replace("+", "%2b"));
if (!request.params().contains(k)) {
params.put(k, value);
} else {
context.pathParams().put(k, value);
}
}
} catch (UnsupportedEncodingException e) {
context.fail(e);
return false;
}
}
request.params().addAll(params);
Expand Down
6 changes: 3 additions & 3 deletions vertx-web/src/main/java/io/vertx/ext/web/impl/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ public static String normalisePath(String path, boolean urldecode) {
for (int i = 0; i < path.length(); i++) {
char c = path.charAt(i);

if (c == '+') {
result.append(' ');
} else if (c == '/') {
// we explicitly ignore the + sign as it should not be translated to
// space within a path as per RFC3986 we only consider percent encoded values
if (c == '/') {
if (i == 0 || result.charAt(result.length() - 1) != '/')
result.append(c);
} else if (urldecode && c == '%') {
Expand Down
11 changes: 11 additions & 0 deletions vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1844,4 +1844,15 @@ public void testParamFirst() throws Exception {
assertTrue(headers.contains("X-Here-2"));
}, 200, "OK", null);
}

@Test
public void testGetWithPlusPath2() throws Exception {
router.get("/:param1").useNormalisedPath(false).handler(rc -> {
assertEquals("/some+path",rc.normalisedPath());
assertEquals("some+path",rc.pathParam("param1"));
assertEquals("some query",rc.request().getParam("q1"));
rc.response().setStatusMessage("foo").end();
});
testRequest(HttpMethod.GET, "/some+path?q1=some+query", 200, "foo");
}
}
3 changes: 2 additions & 1 deletion vertx-web/src/test/java/io/vertx/ext/web/UtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ public void testNullPath() throws Exception {

@Test
public void testPathWithSpaces1() throws Exception {
assertEquals("/foo blah/eek", Utils.normalisePath("/foo+blah/eek"));
// this is a special case since only percent encoded values should be unescaped from the path
assertEquals("/foo+blah/eek", Utils.normalisePath("/foo+blah/eek"));
}

@Test
Expand Down