Skip to content

Improve URI/query strings sanitization #26012

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

Merged
merged 1 commit into from
Nov 4, 2020
Merged

Improve URI/query strings sanitization #26012

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
8 changes: 4 additions & 4 deletions spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -83,7 +83,7 @@ public static ParsedSql parseSqlStatement(final String sql) {
Assert.notNull(sql, "SQL must not be null");

Set<String> namedParameters = new HashSet<>();
String sqlToUse = sql;
StringBuilder sqlToUse = new StringBuilder(sql);
List<ParameterHolder> parameterList = new ArrayList<>();

char[] statement = sql.toCharArray();
Expand Down Expand Up @@ -155,7 +155,7 @@ public static ParsedSql parseSqlStatement(final String sql) {
int j = i + 1;
if (j < statement.length && statement[j] == ':') {
// escaped ":" should be skipped
sqlToUse = sqlToUse.substring(0, i - escapes) + sqlToUse.substring(i - escapes + 1);
sqlToUse.deleteCharAt(i - escapes);
escapes++;
i = i + 2;
continue;
Expand All @@ -174,7 +174,7 @@ public static ParsedSql parseSqlStatement(final String sql) {
}
i++;
}
ParsedSql parsedSql = new ParsedSql(sqlToUse);
ParsedSql parsedSql = new ParsedSql(sqlToUse.toString());
for (ParameterHolder ph : parameterList) {
parsedSql.addNamedParameter(ph.getParameterName(), ph.getStartIndex(), ph.getEndIndex());
}
Expand Down
7 changes: 3 additions & 4 deletions spring-r2dbc/src/main/java/org/springframework/r2dbc/core/NamedParameterUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public static ParsedSql parseSqlStatement(String sql) {
Assert.notNull(sql, "SQL must not be null");

Set<String> namedParameters = new HashSet<>();
String sqlToUse = sql;
StringBuilder sqlToUse = new StringBuilder(sql);
List<ParameterHolder> parameterList = new ArrayList<>();

char[] statement = sql.toCharArray();
Expand Down Expand Up @@ -171,8 +171,7 @@ public static ParsedSql parseSqlStatement(String sql) {
int j = i + 1;
if (j < statement.length && statement[j] == ':') {
// escaped ":" should be skipped
sqlToUse = sqlToUse.substring(0, i - escapes)
+ sqlToUse.substring(i - escapes + 1);
sqlToUse.deleteCharAt(i - escapes);
escapes++;
i = i + 2;
continue;
Expand All @@ -181,7 +180,7 @@ public static ParsedSql parseSqlStatement(String sql) {
}
i++;
}
ParsedSql parsedSql = new ParsedSql(sqlToUse);
ParsedSql parsedSql = new ParsedSql(sqlToUse.toString());
for (ParameterHolder ph : parameterList) {
parsedSql.addNamedParameter(ph.getParameterName(), ph.getStartIndex(), ph.getEndIndex());
}
Expand Down
20 changes: 13 additions & 7 deletions spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1023,15 +1023,21 @@ public PathComponent build() {
if (this.path.length() == 0) {
return null;
}
String path = this.path.toString();
while (true) {
int index = path.indexOf("//");
if (index == -1) {
break;
String sanitized = getSanitizedPath(this.path);
return new HierarchicalUriComponents.FullPathComponent(sanitized);
}

private static String getSanitizedPath(final StringBuilder path) {
int index = path.indexOf("//");
if (index >= 0) {
StringBuilder sanitized = new StringBuilder(path);
while (index != -1) {
sanitized.deleteCharAt(index);
index = sanitized.indexOf("//", index);
}
path = path.substring(0, index) + path.substring(index + 1);
return sanitized.toString();
}
return new HierarchicalUriComponents.FullPathComponent(path);
return path.toString();
}

public void removeTrailingSlash() {
Expand Down
34 changes: 19 additions & 15 deletions spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,18 +401,17 @@ else if (index1 == requestUri.length()) {
* <li>replace all "//" by "/"</li>
* </ul>
*/
private String getSanitizedPath(final String path) {
String sanitized = path;
while (true) {
int index = sanitized.indexOf("//");
if (index < 0) {
break;
}
else {
sanitized = sanitized.substring(0, index) + sanitized.substring(index + 1);
private static String getSanitizedPath(final String path) {
int index = path.indexOf("//");
if (index >= 0) {
StringBuilder sanitized = new StringBuilder(path);
while (index != -1) {
sanitized.deleteCharAt(index);
index = sanitized.indexOf("//", index);
}
return sanitized.toString();
}
return sanitized;
return path;
}

/**
Expand Down Expand Up @@ -612,15 +611,20 @@ public String removeSemicolonContent(String requestUri) {
removeSemicolonContentInternal(requestUri) : removeJsessionid(requestUri));
}

private String removeSemicolonContentInternal(String requestUri) {
private static String removeSemicolonContentInternal(String requestUri) {
int semicolonIndex = requestUri.indexOf(';');
if (semicolonIndex == -1) {
return requestUri;
}
StringBuilder sb = new StringBuilder(requestUri);
while (semicolonIndex != -1) {
int slashIndex = requestUri.indexOf('/', semicolonIndex);
String start = requestUri.substring(0, semicolonIndex);
requestUri = (slashIndex != -1) ? start + requestUri.substring(slashIndex) : start;
semicolonIndex = requestUri.indexOf(';', semicolonIndex);
if (slashIndex >= 0) {
sb.delete(semicolonIndex, slashIndex);
}
semicolonIndex = sb.indexOf(";", semicolonIndex);
}
return requestUri;
return sb.toString();
}

private String removeJsessionid(String requestUri) {
Expand Down
5 changes: 5 additions & 0 deletions spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1200,4 +1200,9 @@ void toUriStringWithCurlyBraces() {
assertThat(UriComponentsBuilder.fromUriString("/path?q={asa}asa").toUriString()).isEqualTo("/path?q=%7Basa%7Dasa");
}

@Test
void verifyDoubleSlashReplacedWithSingleOne() {
String path = UriComponentsBuilder.fromPath("/home/").path("/path").build().getPath();
assertThat(path).isEqualTo("/home/path");
}
}
3 changes: 3 additions & 0 deletions spring-web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ public void getRequestUri() {

request.setRequestURI("/foo+bar");
assertThat(helper.getRequestUri(request)).isEqualTo("/foo+bar");

request.setRequestURI("/home/" + "/path");
assertThat(helper.getRequestUri(request)).isEqualTo("/home/path");
}

@Test
Expand Down