From 0015fd67347b523411a7f34a119b3ffdcbbb9d1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A1=D0=B5=D1=80=D0=B3=D0=B5=D0=B9=20=D0=A6=D1=8B=D0=BF?= =?UTF-8?q?=D0=B0=D0=BD=D0=BE=D0=B2?= Date: Mon, 2 Nov 2020 12:49:00 +0200 Subject: [PATCH] Improve URI/query strings sanitization --- .../core/namedparam/NamedParameterUtils.java | 8 ++--- .../r2dbc/core/NamedParameterUtils.java | 7 ++-- .../web/util/UriComponentsBuilder.java | 20 +++++++---- .../web/util/UrlPathHelper.java | 34 +++++++++++-------- .../web/util/UriComponentsBuilderTests.java | 5 +++ .../web/util/UrlPathHelperTests.java | 3 ++ 6 files changed, 47 insertions(+), 30 deletions(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterUtils.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterUtils.java index 068d4d01ac0..23feefd6738 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterUtils.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/namedparam/NamedParameterUtils.java @@ -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. @@ -83,7 +83,7 @@ public abstract class NamedParameterUtils { Assert.notNull(sql, "SQL must not be null"); Set namedParameters = new HashSet<>(); - String sqlToUse = sql; + StringBuilder sqlToUse = new StringBuilder(sql); List parameterList = new ArrayList<>(); char[] statement = sql.toCharArray(); @@ -155,7 +155,7 @@ public abstract class NamedParameterUtils { 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; @@ -174,7 +174,7 @@ public abstract class NamedParameterUtils { } i++; } - ParsedSql parsedSql = new ParsedSql(sqlToUse); + ParsedSql parsedSql = new ParsedSql(sqlToUse.toString()); for (ParameterHolder ph : parameterList) { parsedSql.addNamedParameter(ph.getParameterName(), ph.getStartIndex(), ph.getEndIndex()); } diff --git a/spring-r2dbc/src/main/java/org/springframework/r2dbc/core/NamedParameterUtils.java b/spring-r2dbc/src/main/java/org/springframework/r2dbc/core/NamedParameterUtils.java index 96a5be28322..293d8451885 100644 --- a/spring-r2dbc/src/main/java/org/springframework/r2dbc/core/NamedParameterUtils.java +++ b/spring-r2dbc/src/main/java/org/springframework/r2dbc/core/NamedParameterUtils.java @@ -95,7 +95,7 @@ abstract class NamedParameterUtils { Assert.notNull(sql, "SQL must not be null"); Set namedParameters = new HashSet<>(); - String sqlToUse = sql; + StringBuilder sqlToUse = new StringBuilder(sql); List parameterList = new ArrayList<>(); char[] statement = sql.toCharArray(); @@ -171,8 +171,7 @@ abstract class NamedParameterUtils { 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; @@ -181,7 +180,7 @@ abstract class NamedParameterUtils { } i++; } - ParsedSql parsedSql = new ParsedSql(sqlToUse); + ParsedSql parsedSql = new ParsedSql(sqlToUse.toString()); for (ParameterHolder ph : parameterList) { parsedSql.addNamedParameter(ph.getParameterName(), ph.getStartIndex(), ph.getEndIndex()); } diff --git a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java index c57f77bc505..e22a8d7c2fd 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java +++ b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java @@ -1023,15 +1023,21 @@ public class UriComponentsBuilder implements UriBuilder, Cloneable { 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() { diff --git a/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java b/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java index 1e8abf7b6b3..493535e09ce 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java +++ b/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java @@ -401,18 +401,17 @@ public class UrlPathHelper { *
  • replace all "//" by "/"
    • * */ - 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; } /** @@ -612,15 +611,20 @@ public class UrlPathHelper { 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) { diff --git a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java index c22e21753e8..fcb71bf37c1 100644 --- a/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/UriComponentsBuilderTests.java @@ -1200,4 +1200,9 @@ class UriComponentsBuilderTests { 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"); + } } diff --git a/spring-web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java b/spring-web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java index 8e88c974ddc..235d7fe4284 100644 --- a/spring-web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/UrlPathHelperTests.java @@ -104,6 +104,9 @@ public class UrlPathHelperTests { request.setRequestURI("/foo+bar"); assertThat(helper.getRequestUri(request)).isEqualTo("/foo+bar"); + + request.setRequestURI("/home/" + "/path"); + assertThat(helper.getRequestUri(request)).isEqualTo("/home/path"); } @Test