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.

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

Improve URI/query strings sanitization #26012

Merged
merged 1 commit into from Nov 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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
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
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
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
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");
}
}
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