diff --git a/.github/workflows/build-and-deploy-snapshot.yml b/.github/workflows/build-and-deploy-snapshot.yml index 4d0e8ad291c..f59a59e769a 100644 --- a/.github/workflows/build-and-deploy-snapshot.yml +++ b/.github/workflows/build-and-deploy-snapshot.yml @@ -2,7 +2,7 @@ name: Build and Deploy Snapshot on: push: branches: - - main + - 'main' concurrency: group: ${{ github.workflow }}-${{ github.ref }} jobs: diff --git a/framework-docs/modules/ROOT/pages/appendix.adoc b/framework-docs/modules/ROOT/pages/appendix.adoc index 8c2cbd703ff..9107fe9c5a2 100644 --- a/framework-docs/modules/ROOT/pages/appendix.adoc +++ b/framework-docs/modules/ROOT/pages/appendix.adoc @@ -74,6 +74,12 @@ expressions used in XML bean definitions, `@Value`, etc. | The mode to use when compiling expressions for the xref:core/expressions/evaluation.adoc#expressions-compiler-configuration[Spring Expression Language]. +| `spring.expression.maxBigPowerBits` +| The default maximum number of bits permitted in the result of a `BigDecimal` or +`BigInteger` power operation within a +xref:core/expressions/evaluation.adoc#expressions-parser-configuration[Spring Expression Language] +expression. + | `spring.expression.maxOperations` | The default maximum number of operations permitted during xref:core/expressions/evaluation.adoc#expressions-parser-configuration[Spring Expression Language] diff --git a/framework-docs/modules/ROOT/pages/core/expressions/evaluation.adoc b/framework-docs/modules/ROOT/pages/core/expressions/evaluation.adoc index cb046c10a79..ef3668351bb 100644 --- a/framework-docs/modules/ROOT/pages/core/expressions/evaluation.adoc +++ b/framework-docs/modules/ROOT/pages/core/expressions/evaluation.adoc @@ -574,6 +574,19 @@ property or Spring property named `spring.expression.maxOperations` to the maxim of operations required by your application (see xref:appendix.adoc#appendix-spring-properties[Supported Spring Properties]). +In addition, the result of a `BigDecimal` or `BigInteger` power operation within a SpEL +expression cannot exceed 1,000,000 bits by default – approximately equivalent to a +decimal number with 300,000 digits. Power operations involving large base values or large +exponents can be computationally expensive, and this limit ensures that evaluations +remain bounded; however, the `maximumBigPowerBits` value is configurable. If you create a +`SpelExpressionParser` programmatically (the recommended approach), you can specify a +custom `maximumBigPowerBits` value when creating the `SpelParserConfiguration` that you +provide to the `SpelExpressionParser`. To remove this limit entirely, pass +`Integer.MAX_VALUE` as the `maximumBigPowerBits` value. If you are not able to configure +an explicit value for `maximumBigPowerBits` via `SpelParserConfiguration`, you can set a +JVM system property or Spring property named `spring.expression.maxBigPowerBits` to the +maximum result size in bits (see xref:appendix.adoc#appendix-spring-properties[Supported +Spring Properties]). [[expressions-spel-compilation]] == SpEL Compilation diff --git a/framework-platform/framework-platform.gradle b/framework-platform/framework-platform.gradle index 8a72bad61c3..38bc2078027 100644 --- a/framework-platform/framework-platform.gradle +++ b/framework-platform/framework-platform.gradle @@ -8,9 +8,9 @@ javaPlatform { dependencies { api(platform("com.fasterxml.jackson:jackson-bom:2.21.5")) - api(platform("io.micrometer:micrometer-bom:1.18.0-SNAPSHOT")) + api(platform("io.micrometer:micrometer-bom:1.18.0-M1")) api(platform("io.netty:netty-bom:4.2.17.Final")) - api(platform("io.projectreactor:reactor-bom:2026.0.0-SNAPSHOT")) + api(platform("io.projectreactor:reactor-bom:2026.0.0-M1")) api(platform("io.rsocket:rsocket-bom:1.1.5")) api(platform("org.apache.groovy:groovy-bom:5.0.8")) api(platform("org.apache.logging.log4j:log4j-bom:2.26.1")) diff --git a/spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java b/spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java index 8253a9fe9cd..4c763b480f5 100644 --- a/spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java @@ -632,6 +632,11 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA else if (value instanceof List list) { int index = Integer.parseInt(key); growCollectionIfNecessary(list, index, indexedPropertyName.toString(), ph, i + 1); + if (index < 0 || index >= list.size()) { + throw new InvalidPropertyException(getRootClass(), this.nestedPath + propertyName, + "Cannot get element with index " + index + " from List of size " + + list.size() + ", accessed using property path '" + propertyName + "'"); + } value = list.get(index); } else if (value instanceof Map map) { diff --git a/spring-beans/src/test/java/org/springframework/beans/BeanWrapperAutoGrowingTests.java b/spring-beans/src/test/java/org/springframework/beans/BeanWrapperAutoGrowingTests.java index 2b3480404fa..33194ed8400 100644 --- a/spring-beans/src/test/java/org/springframework/beans/BeanWrapperAutoGrowingTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/BeanWrapperAutoGrowingTests.java @@ -16,6 +16,7 @@ package org.springframework.beans; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -183,7 +184,34 @@ class BeanWrapperAutoGrowingTests { wrapper.setAutoGrowCollectionLimit(2); assertThatExceptionOfType(InvalidPropertyException.class) .isThrownBy(() -> wrapper.getPropertyValue("list[4]")) - .withRootCauseInstanceOf(IndexOutOfBoundsException.class); + .withMessageContainingAll( + "Invalid property 'list[4]'", + "Cannot get element with index 4 from List of size 0"); + } + + @Test + void getPropertyValueSelfPopulatingListWorksWithinLimit() { + bean.setList(new SelfPopulatingList()); + assertThat(wrapper.getPropertyValue("list[2]")).isInstanceOf(Bean.class); + assertThat(bean.getList()) + .hasSize(3) + .allSatisfy(entry -> assertThat(entry).isInstanceOf(Bean.class)); + } + + @Test + void getPropertyValueSelfPopulatingListFailsAgainstLimit() { + bean.setList(new SelfPopulatingList()); + wrapper.setAutoGrowCollectionLimit(2); + assertThatExceptionOfType(InvalidPropertyException.class) + .isThrownBy(() -> wrapper.getPropertyValue("list[4]")); + } + + @Test + void setPropertyValueSelfPopulatingListFailsAgainstLimitForNestedPath() { + bean.setList(new SelfPopulatingList()); + wrapper.setAutoGrowCollectionLimit(2); + assertThatExceptionOfType(InvalidPropertyException.class) + .isThrownBy(() -> wrapper.setPropertyValue("list[4].prop", "test")); } @Test @@ -382,4 +410,24 @@ class BeanWrapperAutoGrowingTests { } } + + /** + * A {@link List} implementation that creates elements on demand in {@link #get(int)} + * instead of throwing {@link IndexOutOfBoundsException} for out-of-range indexes. + * + *

Used to verify that {@link BeanWrapperImpl} does not delegate to + * {@link List#get(int)} for indexes beyond the configured auto-grow limit. + */ + @SuppressWarnings("serial") + private static class SelfPopulatingList extends ArrayList { + + @Override + public Bean get(int index) { + while (size() <= index) { + add(new Bean()); + } + return super.get(index); + } + } + } diff --git a/spring-context-support/src/main/java/org/springframework/ui/freemarker/SpringTemplateLoader.java b/spring-context-support/src/main/java/org/springframework/ui/freemarker/SpringTemplateLoader.java index 0ea01d1afe8..cf77794e610 100644 --- a/spring-context-support/src/main/java/org/springframework/ui/freemarker/SpringTemplateLoader.java +++ b/spring-context-support/src/main/java/org/springframework/ui/freemarker/SpringTemplateLoader.java @@ -72,6 +72,9 @@ public class SpringTemplateLoader implements TemplateLoader { if (logger.isDebugEnabled()) { logger.debug("Looking for FreeMarker template with name [" + name + "]"); } + if (name.indexOf('\\') != -1) { + return null; + } Resource resource = this.resourceLoader.getResource(this.templateLoaderPath + name); return (resource.exists() ? resource : null); } diff --git a/spring-context-support/src/test/java/org/springframework/ui/freemarker/SpringTemplateLoaderTests.java b/spring-context-support/src/test/java/org/springframework/ui/freemarker/SpringTemplateLoaderTests.java new file mode 100644 index 00000000000..5e7a127575e --- /dev/null +++ b/spring-context-support/src/test/java/org/springframework/ui/freemarker/SpringTemplateLoaderTests.java @@ -0,0 +1,55 @@ +/* + * Copyright 2002-present 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.ui.freemarker; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import org.springframework.core.io.DefaultResourceLoader; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link SpringTemplateLoader}. + * + * @author Sébastien Deleuze + */ +class SpringTemplateLoaderTests { + + @Test + void findTemplateSourceResolvesTemplateInsidePath(@TempDir Path tempDir) throws Exception { + Path templates = Files.createDirectory(tempDir.resolve("templates")); + Files.writeString(templates.resolve("hello.ftl"), "Hello"); + SpringTemplateLoader loader = new SpringTemplateLoader(new DefaultResourceLoader(), + "file:" + templates.toAbsolutePath() + File.separator); + assertThat(loader.findTemplateSource("hello.ftl")).isNotNull(); + } + + @Test + void findTemplateSourceRejectsBackslash(@TempDir Path tempDir) throws Exception { + Path templates = Files.createDirectory(tempDir.resolve("templates")); + Files.writeString(tempDir.resolve("other.txt"), "other"); + SpringTemplateLoader loader = new SpringTemplateLoader(new DefaultResourceLoader(), + "file:" + templates.toAbsolutePath() + File.separator); + assertThat(loader.findTemplateSource("..\\other.txt")).isNull(); + } + +} diff --git a/spring-core/src/main/java/org/springframework/core/SpringProperties.java b/spring-core/src/main/java/org/springframework/core/SpringProperties.java index bcdc21dd127..57e495dd882 100644 --- a/spring-core/src/main/java/org/springframework/core/SpringProperties.java +++ b/spring-core/src/main/java/org/springframework/core/SpringProperties.java @@ -44,6 +44,7 @@ import org.jspecify.annotations.Nullable; * @see org.springframework.beans.factory.support.DefaultListableBeanFactory#STRICT_LOCKING_PROPERTY_NAME * @see org.springframework.core.env.AbstractEnvironment#IGNORE_GETENV_PROPERTY_NAME * @see org.springframework.expression.spel.SpelParserConfiguration#SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME + * @see org.springframework.expression.spel.SpelParserConfiguration#SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME * @see org.springframework.expression.spel.SpelParserConfiguration#SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME * @see org.springframework.jdbc.core.StatementCreatorUtils#IGNORE_GETPARAMETERTYPE_PROPERTY_NAME * @see org.springframework.jndi.JndiLocatorDelegate#IGNORE_JNDI_PROPERTY_NAME diff --git a/spring-expression/src/main/java/org/springframework/expression/EvaluationContext.java b/spring-expression/src/main/java/org/springframework/expression/EvaluationContext.java index 0f7ef0f7927..005903ca0ee 100644 --- a/spring-expression/src/main/java/org/springframework/expression/EvaluationContext.java +++ b/spring-expression/src/main/java/org/springframework/expression/EvaluationContext.java @@ -185,4 +185,26 @@ public interface EvaluationContext { return true; } + /** + * Determine if compilation is supported within expressions evaluated by this evaluation + * context. + *

This flag is orthogonal to whether compilation has been enabled via + * {@link org.springframework.expression.spel.SpelParserConfiguration} or the + * {@value org.springframework.expression.spel.SpelParserConfiguration#SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME} + * system property. Both must permit compilation for a given expression to be compiled. + *

If this method returns {@code false}, the SpEL compiler will not compile + * expressions evaluated within this context, regardless of the compiler mode + * configured in the associated {@code SpelParserConfiguration} or system property. + * Furthermore, if a compiled form of the expression already exists, it will not be + * used; instead, interpreted evaluation will be performed. + *

By default, this method returns {@code true}. Concrete implementations may override + * this default method to indicate that compilation is not supported. + * @return {@code true} if compilation is supported; {@code false} otherwise + * @since 7.0.9 + * @see org.springframework.expression.spel.support.SimpleEvaluationContext.Builder#withCompilationSupported() + */ + default boolean isCompilationSupported() { + return true; + } + } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java b/spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java index d58c89e7aea..28cea823c73 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java @@ -307,7 +307,12 @@ public enum SpelMessage { /** @since 6.2.19 */ MAX_OPERATIONS_EXCEEDED(Kind.ERROR, 1085, - "SpEL expression evaluation exceeded the threshold of ''{0}'' operations"); + "SpEL expression evaluation exceeded the threshold of ''{0}'' operations"), + + /** @since 7.0.9 */ + MAX_BIG_POWER_RESULT_EXCEEDED(Kind.ERROR, 1086, + "BigDecimal/BigInteger power operation with base bit length ''{0}'' and exponent ''{1}'' " + + "would produce a result exceeding the configured maximum of ''{2}'' bits"); private final Kind kind; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/SpelParserConfiguration.java b/spring-expression/src/main/java/org/springframework/expression/spel/SpelParserConfiguration.java index aa2a3db8c74..73e218ec5f8 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/SpelParserConfiguration.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/SpelParserConfiguration.java @@ -49,6 +49,16 @@ public class SpelParserConfiguration { */ public static final int DEFAULT_MAX_OPERATIONS = 10_000; + /** + * Default maximum number of bits permitted in the result of a + * {@link java.math.BigDecimal} or {@link java.math.BigInteger} power operation + * within a SpEL expression: {@value}. + *

Approximately equivalent to a decimal number with 300,000 digits. + * @since 7.0.9 + * @see #SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME + */ + public static final int DEFAULT_MAX_BIG_POWER_BITS = 1_000_000; + /** * System property to configure the default compiler mode for SpEL expression parsers: {@value}. *

NOTE: Instead of relying on a global default, applications @@ -65,7 +75,7 @@ public class SpelParserConfiguration { * during SpEL expression evaluation: {@value}. *

NOTE: Instead of relying on a global default, applications * and frameworks should ideally set an explicit custom value via the - * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int, int)} * constructor which provides complete configuration control and the ability * to override global defaults per use case. *

Can also be configured via the {@link SpringProperties} mechanism. @@ -74,6 +84,22 @@ public class SpelParserConfiguration { */ public static final String SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME = "spring.expression.maxOperations"; + /** + * System property to configure the default maximum number of bits permitted in the + * result of a {@link java.math.BigDecimal} or {@link java.math.BigInteger} power + * operation within a SpEL expression: {@value}. + *

NOTE: Instead of relying on a global default, applications + * and frameworks should ideally set an explicit custom value via the + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int, int)} + * constructor which provides complete configuration control and the ability + * to override global defaults per use case. + *

Can also be configured via the {@link SpringProperties} mechanism. + * @since 7.0.9 + * @see #DEFAULT_MAX_BIG_POWER_BITS + */ + public static final String SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME = + "spring.expression.maxBigPowerBits"; + private static final SpelCompilerMode defaultCompilerMode; @@ -98,15 +124,18 @@ public class SpelParserConfiguration { private final int maximumOperations; + private final int maximumBigPowerBits; + /** * Create a new {@code SpelParserConfiguration} instance with default settings. *

NOTE: Favor the - * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int, int)} * constructor for complete configuration control and the ability to override * global defaults per use case. * @see #SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME + * @see #SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME */ public SpelParserConfiguration() { this(null, null, false, false, Integer.MAX_VALUE); @@ -115,7 +144,7 @@ public class SpelParserConfiguration { /** * Create a new {@code SpelParserConfiguration} instance. *

NOTE: Favor the - * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int, int)} * constructor for complete configuration control and the ability to override * global defaults per use case. * @param compilerMode the compiler mode that parsers using this configuration @@ -124,6 +153,7 @@ public class SpelParserConfiguration { * expression compilation; or {@code null} to use the default {@code ClassLoader} * @see #SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME + * @see #SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME */ public SpelParserConfiguration(@Nullable SpelCompilerMode compilerMode, @Nullable ClassLoader compilerClassLoader) { this(compilerMode, compilerClassLoader, false, false, Integer.MAX_VALUE); @@ -132,13 +162,14 @@ public class SpelParserConfiguration { /** * Create a new {@code SpelParserConfiguration} instance. *

NOTE: Favor the - * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int, int)} * constructor for complete configuration control and the ability to override * global defaults per use case. * @param autoGrowNullReferences if null references should automatically grow * @param autoGrowCollections if collections should automatically grow * @see #SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME + * @see #SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME */ public SpelParserConfiguration(boolean autoGrowNullReferences, boolean autoGrowCollections) { this(null, null, autoGrowNullReferences, autoGrowCollections, Integer.MAX_VALUE); @@ -147,7 +178,7 @@ public class SpelParserConfiguration { /** * Create a new {@code SpelParserConfiguration} instance. *

NOTE: Favor the - * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int, int)} * constructor for complete configuration control and the ability to override * global defaults per use case. * @param autoGrowNullReferences if null references should automatically grow @@ -155,6 +186,7 @@ public class SpelParserConfiguration { * @param maximumAutoGrowSize the maximum size to which a collection can auto grow * @see #SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME + * @see #SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME */ public SpelParserConfiguration(boolean autoGrowNullReferences, boolean autoGrowCollections, int maximumAutoGrowSize) { this(null, null, autoGrowNullReferences, autoGrowCollections, maximumAutoGrowSize); @@ -163,7 +195,7 @@ public class SpelParserConfiguration { /** * Create a new {@code SpelParserConfiguration} instance. *

NOTE: Favor the - * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int, int)} * constructor for complete configuration control and the ability to override * global defaults per use case. * @param compilerMode the compiler mode that parsers using this configuration @@ -175,6 +207,7 @@ public class SpelParserConfiguration { * @param maximumAutoGrowSize the maximum size to which a collection can auto grow * @see #SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME + * @see #SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME */ public SpelParserConfiguration(@Nullable SpelCompilerMode compilerMode, @Nullable ClassLoader compilerClassLoader, boolean autoGrowNullReferences, boolean autoGrowCollections, int maximumAutoGrowSize) { @@ -186,7 +219,7 @@ public class SpelParserConfiguration { /** * Create a new {@code SpelParserConfiguration} instance. *

NOTE: Favor the - * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int)} + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int, int)} * constructor for complete configuration control and the ability to override * global defaults per use case. * @param compilerMode the compiler mode that parsers using this configuration @@ -201,6 +234,7 @@ public class SpelParserConfiguration { * @since 5.2.25 * @see #SPRING_EXPRESSION_COMPILER_MODE_PROPERTY_NAME * @see #SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME + * @see #SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME */ public SpelParserConfiguration(@Nullable SpelCompilerMode compilerMode, @Nullable ClassLoader compilerClassLoader, boolean autoGrowNullReferences, boolean autoGrowCollections, int maximumAutoGrowSize, int maximumExpressionLength) { @@ -209,6 +243,34 @@ public class SpelParserConfiguration { autoGrowCollections, maximumAutoGrowSize, maximumExpressionLength, retrieveMaxOperations()); } + /** + * Create a new {@code SpelParserConfiguration} instance. + *

NOTE: Favor the + * {@link #SpelParserConfiguration(SpelCompilerMode, ClassLoader, boolean, boolean, int, int, int, int)} + * constructor for complete configuration control and the ability to override + * global defaults per use case. + * @param compilerMode the compiler mode that parsers using this configuration + * should use; must not be {@code null} + * @param compilerClassLoader the {@code ClassLoader} to use as the basis for + * expression compilation; or {@code null} to use the default {@code ClassLoader} + * @param autoGrowNullReferences if null references should automatically grow + * @param autoGrowCollections if collections should automatically grow + * @param maximumAutoGrowSize the maximum size to which a collection can auto grow + * @param maximumExpressionLength the maximum length of a SpEL expression; + * must be a positive number + * @param maximumOperations the maximum number of operations permitted during + * SpEL expression evaluation; must be a positive number + * @since 6.2.19 + * @see #SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME + */ + public SpelParserConfiguration(SpelCompilerMode compilerMode, @Nullable ClassLoader compilerClassLoader, + boolean autoGrowNullReferences, boolean autoGrowCollections, int maximumAutoGrowSize, int maximumExpressionLength, + int maximumOperations) { + + this(compilerMode, compilerClassLoader, autoGrowNullReferences, autoGrowCollections, + maximumAutoGrowSize, maximumExpressionLength, maximumOperations, retrieveMaxBigPowerBits()); + } + /** * Create a new {@code SpelParserConfiguration} instance. * @param compilerMode the compiler mode that parsers using this configuration @@ -222,15 +284,19 @@ public class SpelParserConfiguration { * must be a positive number * @param maximumOperations the maximum number of operations permitted during * SpEL expression evaluation; must be a positive number - * @since 6.2.19 + * @param maximumBigPowerBits the maximum number of bits permitted in the + * result of a {@link java.math.BigDecimal} or {@link java.math.BigInteger} power + * operation; must be a positive number; use {@link Integer#MAX_VALUE} for no limit + * @since 7.0.9 */ public SpelParserConfiguration(SpelCompilerMode compilerMode, @Nullable ClassLoader compilerClassLoader, boolean autoGrowNullReferences, boolean autoGrowCollections, int maximumAutoGrowSize, int maximumExpressionLength, - int maximumOperations) { + int maximumOperations, int maximumBigPowerBits) { Assert.notNull(compilerMode, "'compilerMode' must not be null"); Assert.isTrue(maximumExpressionLength > 0, "'maximumExpressionLength' must be a positive number"); Assert.isTrue(maximumOperations > 0, "'maximumOperations' must be a positive number"); + Assert.isTrue(maximumBigPowerBits > 0, "'maximumBigPowerBits' must be a positive number"); this.compilerMode = compilerMode; this.compilerClassLoader = compilerClassLoader; @@ -239,6 +305,7 @@ public class SpelParserConfiguration { this.maximumAutoGrowSize = maximumAutoGrowSize; this.maximumExpressionLength = maximumExpressionLength; this.maximumOperations = maximumOperations; + this.maximumBigPowerBits = maximumBigPowerBits; } @@ -294,6 +361,15 @@ public class SpelParserConfiguration { return this.maximumOperations; } + /** + * Return the maximum number of bits permitted in the result of a + * {@link java.math.BigDecimal} or {@link java.math.BigInteger} power operation. + * @since 7.0.9 + */ + public int getMaximumBigPowerBits() { + return this.maximumBigPowerBits; + } + private static int retrieveMaxOperations() { String value = SpringProperties.getProperty(SPRING_EXPRESSION_MAX_OPERATIONS_PROPERTY_NAME); @@ -313,4 +389,21 @@ public class SpelParserConfiguration { } } + private static int retrieveMaxBigPowerBits() { + String value = SpringProperties.getProperty(SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME); + if (!StringUtils.hasText(value)) { + return DEFAULT_MAX_BIG_POWER_BITS; + } + try { + int maxBits = Integer.parseInt(value.trim()); + Assert.isTrue(maxBits > 0, () -> "Value [" + maxBits + "] for system property [" + + SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME + "] must be positive"); + return maxBits; + } + catch (NumberFormatException ex) { + throw new IllegalArgumentException("Failed to parse value for system property [" + + SPRING_EXPRESSION_MAX_BIG_POWER_BITS_PROPERTY_NAME + "]: " + ex.getMessage(), ex); + } + } + } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorPower.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorPower.java index 1891db890f6..ac7e4eeadce 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorPower.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorPower.java @@ -23,13 +23,15 @@ import org.springframework.expression.EvaluationException; import org.springframework.expression.Operation; import org.springframework.expression.TypedValue; import org.springframework.expression.spel.ExpressionState; -import org.springframework.util.NumberUtils; +import org.springframework.expression.spel.SpelEvaluationException; +import org.springframework.expression.spel.SpelMessage; /** * The power operator. * * @author Andy Clement * @author Giovanni Dall'Oglio Risso + * @author Sam Brannen * @since 3.0 */ public class OperatorPower extends Operator { @@ -41,21 +43,20 @@ public class OperatorPower extends Operator { @Override public TypedValue getValueInternal(ExpressionState state) throws EvaluationException { - SpelNodeImpl leftOp = getLeftOperand(); - SpelNodeImpl rightOp = getRightOperand(); - - Object leftOperand = leftOp.getValueInternal(state).getValue(); - Object rightOperand = rightOp.getValueInternal(state).getValue(); + Object leftOperand = getLeftOperand().getValueInternal(state).getValue(); + Object rightOperand = getRightOperand().getValueInternal(state).getValue(); if (leftOperand instanceof Number leftNumber && rightOperand instanceof Number rightNumber) { state.trackOperation(); - if (leftNumber instanceof BigDecimal) { - BigDecimal leftBigDecimal = NumberUtils.convertNumberToTargetClass(leftNumber, BigDecimal.class); - return new TypedValue(leftBigDecimal.pow(rightNumber.intValue())); + if (leftNumber instanceof BigDecimal leftBigDecimal) { + int exponent = rightNumber.intValue(); + checkBigNumberPowerBits(state, leftBigDecimal.unscaledValue().bitLength(), exponent); + return new TypedValue(leftBigDecimal.pow(exponent)); } - else if (leftNumber instanceof BigInteger) { - BigInteger leftBigInteger = NumberUtils.convertNumberToTargetClass(leftNumber, BigInteger.class); - return new TypedValue(leftBigInteger.pow(rightNumber.intValue())); + else if (leftNumber instanceof BigInteger leftBigInteger) { + int exponent = rightNumber.intValue(); + checkBigNumberPowerBits(state, leftBigInteger.bitLength(), exponent); + return new TypedValue(leftBigInteger.pow(exponent)); } else if (leftNumber instanceof Double || rightNumber instanceof Double) { return new TypedValue(Math.pow(leftNumber.doubleValue(), rightNumber.doubleValue())); @@ -76,4 +77,13 @@ public class OperatorPower extends Operator { return state.operate(Operation.POWER, leftOperand, rightOperand); } + private void checkBigNumberPowerBits(ExpressionState state, int baseBitLength, int exponent) { + int maxBigPowerBits = state.getConfiguration().getMaximumBigPowerBits(); + long estimatedBigPowerBits = (long) baseBitLength * exponent; + if (estimatedBigPowerBits > maxBigPowerBits) { + throw new SpelEvaluationException(getStartPosition(), SpelMessage.MAX_BIG_POWER_RESULT_EXCEEDED, + baseBitLength, exponent, maxBigPowerBits); + } + } + } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java b/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java index 87133da0779..6664f17f358 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java @@ -75,14 +75,14 @@ public class SpelExpression implements Expression { private final SpelParserConfiguration configuration; - // The default context is used if no override is supplied by the user + // The default context is used if no override is supplied by the user. private @Nullable EvaluationContext evaluationContext; - // Holds the compiled form of the expression (if it has been compiled) + // Holds the compiled form of the expression (if it has been compiled). private volatile @Nullable CompiledExpression compiledAst; - // Count of many times as the expression been interpreted - can trigger compilation - // when certain limit reached + // Counts how many times the expression has been interpreted - can trigger compilation + // when a certain limit is reached. private final AtomicInteger interpretedCount = new AtomicInteger(); // The number of times compilation was attempted and failed - enables us to eventually @@ -129,10 +129,10 @@ public class SpelExpression implements Expression { @Override public @Nullable Object getValue() throws EvaluationException { + EvaluationContext context = getEvaluationContext(); CompiledExpression compiledAst = this.compiledAst; - if (compiledAst != null) { + if (compiledAst != null && context.isCompilationSupported()) { try { - EvaluationContext context = getEvaluationContext(); return compiledAst.getValue(context.getRootObject().getValue(), context); } catch (Throwable ex) { @@ -148,7 +148,7 @@ public class SpelExpression implements Expression { } } - ExpressionState expressionState = new ExpressionState(getEvaluationContext(), this.configuration); + ExpressionState expressionState = new ExpressionState(context, this.configuration); Object result = this.ast.getValue(expressionState); checkCompile(expressionState); return result; @@ -157,17 +157,16 @@ public class SpelExpression implements Expression { @SuppressWarnings("unchecked") @Override public @Nullable T getValue(@Nullable Class expectedResultType) throws EvaluationException { + EvaluationContext context = getEvaluationContext(); CompiledExpression compiledAst = this.compiledAst; - if (compiledAst != null) { + if (compiledAst != null && context.isCompilationSupported()) { try { - EvaluationContext context = getEvaluationContext(); Object result = compiledAst.getValue(context.getRootObject().getValue(), context); if (expectedResultType == null) { return (T) result; } else { - return ExpressionUtils.convertTypedValue( - getEvaluationContext(), new TypedValue(result), expectedResultType); + return ExpressionUtils.convertTypedValue(context, new TypedValue(result), expectedResultType); } } catch (Throwable ex) { @@ -183,19 +182,19 @@ public class SpelExpression implements Expression { } } - ExpressionState expressionState = new ExpressionState(getEvaluationContext(), this.configuration); + ExpressionState expressionState = new ExpressionState(context, this.configuration); TypedValue typedResultValue = this.ast.getTypedValue(expressionState); checkCompile(expressionState); - return ExpressionUtils.convertTypedValue( - expressionState.getEvaluationContext(), typedResultValue, expectedResultType); + return ExpressionUtils.convertTypedValue(context, typedResultValue, expectedResultType); } @Override public @Nullable Object getValue(@Nullable Object rootObject) throws EvaluationException { + EvaluationContext context = getEvaluationContext(); CompiledExpression compiledAst = this.compiledAst; - if (compiledAst != null) { + if (compiledAst != null && context.isCompilationSupported()) { try { - return compiledAst.getValue(rootObject, getEvaluationContext()); + return compiledAst.getValue(rootObject, context); } catch (Throwable ex) { // If running in mixed mode, revert to interpreted @@ -211,7 +210,7 @@ public class SpelExpression implements Expression { } ExpressionState expressionState = - new ExpressionState(getEvaluationContext(), toTypedValue(rootObject), this.configuration); + new ExpressionState(context, toTypedValue(rootObject), this.configuration); Object result = this.ast.getValue(expressionState); checkCompile(expressionState); return result; @@ -220,16 +219,16 @@ public class SpelExpression implements Expression { @SuppressWarnings("unchecked") @Override public @Nullable T getValue(@Nullable Object rootObject, @Nullable Class expectedResultType) throws EvaluationException { + EvaluationContext context = getEvaluationContext(); CompiledExpression compiledAst = this.compiledAst; - if (compiledAst != null) { + if (compiledAst != null && context.isCompilationSupported()) { try { - Object result = compiledAst.getValue(rootObject, getEvaluationContext()); + Object result = compiledAst.getValue(rootObject, context); if (expectedResultType == null) { - return (T)result; + return (T) result; } else { - return ExpressionUtils.convertTypedValue( - getEvaluationContext(), new TypedValue(result), expectedResultType); + return ExpressionUtils.convertTypedValue(context, new TypedValue(result), expectedResultType); } } catch (Throwable ex) { @@ -246,11 +245,10 @@ public class SpelExpression implements Expression { } ExpressionState expressionState = - new ExpressionState(getEvaluationContext(), toTypedValue(rootObject), this.configuration); + new ExpressionState(context, toTypedValue(rootObject), this.configuration); TypedValue typedResultValue = this.ast.getTypedValue(expressionState); checkCompile(expressionState); - return ExpressionUtils.convertTypedValue( - expressionState.getEvaluationContext(), typedResultValue, expectedResultType); + return ExpressionUtils.convertTypedValue(context, typedResultValue, expectedResultType); } @Override @@ -258,7 +256,7 @@ public class SpelExpression implements Expression { Assert.notNull(context, "EvaluationContext must not be null"); CompiledExpression compiledAst = this.compiledAst; - if (compiledAst != null) { + if (compiledAst != null && context.isCompilationSupported()) { try { return compiledAst.getValue(context.getRootObject().getValue(), context); } @@ -287,7 +285,7 @@ public class SpelExpression implements Expression { Assert.notNull(context, "EvaluationContext must not be null"); CompiledExpression compiledAst = this.compiledAst; - if (compiledAst != null) { + if (compiledAst != null && context.isCompilationSupported()) { try { Object result = compiledAst.getValue(context.getRootObject().getValue(), context); if (expectedResultType != null) { @@ -321,7 +319,7 @@ public class SpelExpression implements Expression { Assert.notNull(context, "EvaluationContext must not be null"); CompiledExpression compiledAst = this.compiledAst; - if (compiledAst != null) { + if (compiledAst != null && context.isCompilationSupported()) { try { return compiledAst.getValue(rootObject, context); } @@ -352,7 +350,7 @@ public class SpelExpression implements Expression { Assert.notNull(context, "EvaluationContext must not be null"); CompiledExpression compiledAst = this.compiledAst; - if (compiledAst != null) { + if (compiledAst != null && context.isCompilationSupported()) { try { Object result = compiledAst.getValue(rootObject, context); if (expectedResultType != null) { @@ -480,6 +478,9 @@ public class SpelExpression implements Expression { */ private void checkCompile(ExpressionState expressionState) { this.interpretedCount.incrementAndGet(); + if (!expressionState.getEvaluationContext().isCompilationSupported()) { + return; + } SpelCompilerMode compilerMode = expressionState.getConfiguration().getCompilerMode(); if (compilerMode != SpelCompilerMode.OFF) { if (compilerMode == SpelCompilerMode.IMMEDIATE) { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java index fed658a2fa4..486d01deccb 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java @@ -150,10 +150,12 @@ public final class SimpleEvaluationContext implements EvaluationContext { private final boolean assignmentEnabled; + private final boolean compilationSupported; + private SimpleEvaluationContext(List propertyAccessors, List indexAccessors, List resolvers, @Nullable TypeConverter converter, @Nullable TypedValue rootObject, - boolean assignmentEnabled) { + boolean assignmentEnabled, boolean compilationSupported) { this.propertyAccessors = propertyAccessors; this.indexAccessors = indexAccessors; @@ -161,6 +163,7 @@ public final class SimpleEvaluationContext implements EvaluationContext { this.typeConverter = (converter != null ? converter : new StandardTypeConverter()); this.rootObject = (rootObject != null ? rootObject : TypedValue.NULL); this.assignmentEnabled = assignmentEnabled; + this.compilationSupported = compilationSupported; } @@ -302,6 +305,19 @@ public final class SimpleEvaluationContext implements EvaluationContext { return this.assignmentEnabled; } + /** + * Determine if compilation is supported within expressions evaluated by this evaluation + * context. + *

By default, compilation is not supported in {@code SimpleEvaluationContext}. + * @return {@code true} if compilation is supported; {@code false} otherwise + * @since 7.0.9 + * @see Builder#withCompilationSupported() + */ + @Override + public boolean isCompilationSupported() { + return this.compilationSupported; + } + /** * Create a {@code SimpleEvaluationContext} for the specified {@link PropertyAccessor} * delegates: typically a custom {@code PropertyAccessor} specific to a use case — @@ -375,6 +391,8 @@ public final class SimpleEvaluationContext implements EvaluationContext { private boolean assignmentEnabled = true; + private boolean compilationSupported = false; + private Builder(PropertyAccessor... accessors) { this.propertyAccessors = Arrays.asList(accessors); @@ -391,6 +409,22 @@ public final class SimpleEvaluationContext implements EvaluationContext { return this; } + /** + * Indicate that compilation is supported within expressions evaluated by this + * evaluation context. + *

By default, compilation is not supported in {@code SimpleEvaluationContext}. + * Call this method to opt in to compilation — for example, when evaluating + * trusted expressions where performance is critical. + *

WARNING: Opting in to compilation for expressions from + * untrusted sources removes the safety guards supported in interpreted mode. + * @since 7.0.9 + * @see SimpleEvaluationContext#isCompilationSupported() + */ + public Builder withCompilationSupported() { + this.compilationSupported = true; + return this; + } + /** * Register the specified {@link IndexAccessor} delegates. * @param indexAccessors the index accessors to use @@ -480,7 +514,8 @@ public final class SimpleEvaluationContext implements EvaluationContext { public SimpleEvaluationContext build() { return new SimpleEvaluationContext(this.propertyAccessors, this.indexAccessors, - this.resolvers, this.typeConverter, this.rootObject, this.assignmentEnabled); + this.resolvers, this.typeConverter, this.rootObject, + this.assignmentEnabled, this.compilationSupported); } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/AbstractExpressionTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/AbstractExpressionTests.java index 31a248a08b7..721ed9fd81e 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/AbstractExpressionTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/AbstractExpressionTests.java @@ -19,6 +19,7 @@ package org.springframework.expression.spel; import java.util.Arrays; import java.util.List; +import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; import org.springframework.expression.ExpressionParser; import org.springframework.expression.spel.standard.SpelExpressionParser; @@ -168,6 +169,23 @@ public abstract class AbstractExpressionTests { evaluateAndCheckError(this.parser, expression, expectedReturnType, expectedMessage, otherProperties); } + /** + * Evaluate the specified expression and ensure the expected message comes out. + * The message may have inserts and they will be checked if otherProperties is specified. + * The first entry in otherProperties should always be the position. + * @param evaluationContext the evaluation context to use + * @param expression the expression to evaluate + * @param expectedReturnType ask the expression return value to be of this type if possible + * ({@code null} indicates don't ask for conversion) + * @param expectedMessage the expected message + * @param otherProperties the expected inserts within the message + */ + protected void evaluateAndCheckError(EvaluationContext evaluationContext, String expression, + Class expectedReturnType, SpelMessage expectedMessage, Object... otherProperties) { + + evaluateAndCheckError(this.parser, evaluationContext, expression, expectedReturnType, expectedMessage, otherProperties); + } + /** * Evaluate the specified expression and ensure the expected message comes out. * The message may have inserts and they will be checked if otherProperties is specified. @@ -182,14 +200,32 @@ public abstract class AbstractExpressionTests { protected void evaluateAndCheckError(ExpressionParser parser, String expression, Class expectedReturnType, SpelMessage expectedMessage, Object... otherProperties) { + evaluateAndCheckError(parser, this.context, expression, expectedReturnType, expectedMessage, otherProperties); + } + + /** + * Evaluate the specified expression and ensure the expected message comes out. + * The message may have inserts and they will be checked if otherProperties is specified. + * The first entry in otherProperties should always be the position. + * @param parser the expression parser to use + * @param evaluationContext the evaluation context to use + * @param expression the expression to evaluate + * @param expectedReturnType ask the expression return value to be of this type if possible + * ({@code null} indicates don't ask for conversion) + * @param expectedMessage the expected message + * @param otherProperties the expected inserts within the message + */ + protected void evaluateAndCheckError(ExpressionParser parser, EvaluationContext evaluationContext, + String expression, Class expectedReturnType, SpelMessage expectedMessage, Object... otherProperties) { + assertThatExceptionOfType(SpelEvaluationException.class).isThrownBy(() -> { Expression expr = parser.parseExpression(expression); assertThat(expr).as("expression").isNotNull(); if (expectedReturnType != null) { - expr.getValue(context, expectedReturnType); + expr.getValue(evaluationContext, expectedReturnType); } else { - expr.getValue(context); + expr.getValue(evaluationContext); } }).satisfies(ex -> { assertThat(ex.getMessageCode()).isEqualTo(expectedMessage); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java index 06290373568..494cd7856ac 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java @@ -811,6 +811,77 @@ class EvaluationTests extends AbstractExpressionTests { } + @Nested + class PowerOperatorTests { + + private final EvaluationContext context = SimpleEvaluationContext.forReadOnlyDataBinding().build(); + + // Use a small limit (16 bits) to verify behavior in tests. + private static final int TEST_MAX_RESULT_BITS = 16; + + private final SpelExpressionParser limitedParser = new SpelExpressionParser( + new SpelParserConfiguration(SpelCompilerMode.OFF, null, false, false, + 0, 10, 10, TEST_MAX_RESULT_BITS)); + + + @Test + void powerOperatorWithBigDecimal() { + context.setVariable("bd", BigDecimal.valueOf(2.0)); + Expression expr = parser.parseExpression("#bd ^ 4"); + assertThat(expr.getValue(context, BigDecimal.class)).isEqualByComparingTo("16"); + } + + @Test + void powerOperatorWithBigDecimalUnderResultLimit() { + // BigDecimal.valueOf(2.0).unscaledValue().bitLength() = 5 + // 5 * 3 = 15 bits <= TEST_MAX_RESULT_BITS (16) + context.setVariable("bd", BigDecimal.valueOf(2.0)); + Expression expr = limitedParser.parseExpression("#bd ^ 3"); + assertThat(expr.getValue(context, BigDecimal.class)).isEqualByComparingTo("8"); + } + + @Test + void powerOperatorWithBigDecimalExceedingResultLimit() { + // 5 * 4 = 20 bits > TEST_MAX_RESULT_BITS (16) + context.setVariable("bd", BigDecimal.valueOf(2.0)); + evaluateAndCheckError(limitedParser, context, "#bd ^ 4", BigDecimal.class, + SpelMessage.MAX_BIG_POWER_RESULT_EXCEEDED, + 4, // power operator position + 5, // base bit length + 4, // exponent + TEST_MAX_RESULT_BITS); + } + + @Test + void powerOperatorWithBigInteger() { + context.setVariable("bi", BigInteger.valueOf(2)); + Expression expr = parser.parseExpression("#bi ^ 4"); + assertThat(expr.getValue(context, BigInteger.class)).isEqualTo(BigInteger.valueOf(16)); + } + + @Test + void powerOperatorWithBigIntegerUnderResultLimit() { + // BigInteger.valueOf(2).bitLength() = 2 + // 2 * 8 = 16 bits == TEST_MAX_RESULT_BITS (16) + context.setVariable("bi", BigInteger.valueOf(2)); + Expression expr = limitedParser.parseExpression("#bi ^ 8"); + assertThat(expr.getValue(context, BigInteger.class)).isEqualTo(BigInteger.valueOf(256)); + } + + @Test + void powerOperatorWithBigIntegerExceedingResultLimit() { + // 2 * 9 = 18 bits > TEST_MAX_RESULT_BITS (16) + context.setVariable("bi", BigInteger.valueOf(2)); + evaluateAndCheckError(limitedParser, context, "#bi ^ 9", BigInteger.class, + SpelMessage.MAX_BIG_POWER_RESULT_EXCEEDED, + 4, // power operator position + 2, // base bit length + 9, // exponent + TEST_MAX_RESULT_BITS); + } + + } + @Nested class TernaryOperatorTests { diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelCompilerTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelCompilerTests.java index 65f957d4a2a..67799dd7ab5 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelCompilerTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelCompilerTests.java @@ -21,14 +21,18 @@ import java.util.stream.IntStream; import org.junit.jupiter.api.Test; import org.springframework.core.Ordered; +import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; import org.springframework.expression.spel.SpelCompilerMode; import org.springframework.expression.spel.SpelParserConfiguration; +import org.springframework.expression.spel.support.SimpleEvaluationContext; import org.springframework.expression.spel.support.StandardEvaluationContext; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.InstanceOfAssertFactories.BOOLEAN; import static org.springframework.expression.spel.standard.SpelExpressionTestUtils.assertIsCompiled; +import static org.springframework.expression.spel.standard.SpelExpressionTestUtils.assertIsNotCompiled; +import static org.springframework.expression.spel.standard.SpelExpressionTestUtils.getInterpretedCount; /** * Tests for the {@link SpelCompiler}. @@ -50,6 +54,7 @@ class SpelCompilerTests { // Evaluate the expression multiple times to ensure that it gets compiled. IntStream.rangeClosed(1, 5).forEach(i -> assertThat(expression.getValue(component)).isEqualTo(42)); + assertIsCompiled(expression); } @Test // gh-25706 @@ -77,6 +82,162 @@ class SpelCompilerTests { assertThat(expression.getValue(context)).asInstanceOf(BOOLEAN).isTrue(); } + @Test + void simpleEvaluationContextBlocksCompilationByDefault() { + SpelParserConfiguration config = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null); + SpelExpressionParser parser = new SpelExpressionParser(config); + // "order" resides in the public Ordered interface and is therefore compilable, + // so any non-compilation is attributable solely to the context's policy. + Expression expression = parser.parseExpression("order"); + + SimpleEvaluationContext context = SimpleEvaluationContext.forReadOnlyDataBinding().build(); + assertThat(context.isCompilationSupported()).isFalse(); + + // Evaluate the expression multiple times to ensure that it stays in interpreted mode, + // effectively overriding SpelCompilerMode.IMMEDIATE. + OrderedComponent component = new OrderedComponent(); + IntStream.rangeClosed(1, 5).forEach(i -> assertThat(expression.getValue(context, component)).isEqualTo(42)); + assertIsNotCompiled(expression); + } + + @Test + void simpleEvaluationContextAllowsCompilationWhenSupported() { + SpelParserConfiguration config = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null); + SpelExpressionParser parser = new SpelExpressionParser(config); + // "order" resides in the public Ordered interface and is therefore compilable. + Expression expression = parser.parseExpression("order"); + + SimpleEvaluationContext context = SimpleEvaluationContext.forReadOnlyDataBinding() + .withCompilationSupported() + .build(); + assertThat(context.isCompilationSupported()).isTrue(); + + // Two evaluations are enough for IMMEDIATE mode to compile. + OrderedComponent component = new OrderedComponent(); + IntStream.rangeClosed(1, 2).forEach(i -> assertThat(expression.getValue(context, component)).isEqualTo(42)); + assertIsCompiled(expression); + } + + @Test + void simpleEvaluationContextIgnoresPrecompiledExpressionByDefault() { + SpelParserConfiguration config = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null); + SpelExpressionParser parser = new SpelExpressionParser(config); + // "order" resides in the public Ordered interface and is therefore compilable. + Expression expression = parser.parseExpression("order"); + + EvaluationContext standardContext = new StandardEvaluationContext(); + assertThat(standardContext.isCompilationSupported()).isTrue(); + OrderedComponent component = new OrderedComponent(); + IntStream.rangeClosed(1, 2).forEach(i -> assertThat(expression.getValue(standardContext, component)).isEqualTo(42)); + assertIsCompiled(expression); + + // Switch to a SimpleEvaluationContext without opting into compilation — should + // fall back to interpreted evaluation even though compiledAst is non-null. + EvaluationContext simpleContext = SimpleEvaluationContext.forReadOnlyDataBinding().build(); + assertThat(simpleContext.isCompilationSupported()).isFalse(); + + // Record interpretedCount before the simpleContext evaluation. + // checkCompile() — which increments interpretedCount as its very first action — + // is only reachable from the interpreted path. If the compiled path were taken + // instead, interpretedCount would not change. + int interpretedCountBefore = getInterpretedCount(expression); + assertThat(expression.getValue(simpleContext, component)).isEqualTo(42); + assertThat(getInterpretedCount(expression)).isEqualTo(interpretedCountBefore + 1); + + // compiledAst is still set: the compiled expression was not cleared, rather merely ignored. + assertIsCompiled(expression); + } + + /** + * Verify that the four implicit {@link EvaluationContext} {@code getValue()} variants + * in {@link SpelExpression} honor a {@link SimpleEvaluationContext} set as the default + * context: compilation must be blocked even under {@link SpelCompilerMode#IMMEDIATE}. + */ + @Test + void simpleEvaluationContextSetAsDefaultBlocksCompilationForImplicitContextVariants() { + SpelParserConfiguration config = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null); + SpelExpressionParser parser = new SpelExpressionParser(config); + // "order" resides in the public Ordered interface and is therefore compilable, + // so any non-compilation is attributable solely to the context's policy. + SpelExpression expression = parser.parseRaw("order"); + + OrderedComponent component = new OrderedComponent(); + SimpleEvaluationContext simpleContext = SimpleEvaluationContext.forReadOnlyDataBinding() + .withRootObject(component) + .build(); + assertThat(simpleContext.isCompilationSupported()).isFalse(); + expression.setEvaluationContext(simpleContext); + + // Evaluate the expression multiple times using all four implicit context + // variants to ensure that they stay in interpreted mode. + for (int i = 0; i < 5; i++) { + assertThat(expression.getValue()).isEqualTo(42); + assertIsNotCompiled(expression); + + assertThat(expression.getValue(Integer.class)).isEqualTo(42); + assertIsNotCompiled(expression); + + assertThat(expression.getValue(component)).isEqualTo(42); + assertIsNotCompiled(expression); + + assertThat(expression.getValue(component, Integer.class)).isEqualTo(42); + assertIsNotCompiled(expression); + } + } + + /** + * Verify that the four implicit {@link EvaluationContext} {@code getValue()} variants + * in {@link SpelExpression} ignore a previously compiled expression when the default + * context is a {@link SimpleEvaluationContext} (where {@code isCompilationSupported()} + * returns {@code false}). + */ + @Test + void simpleEvaluationContextSetAsDefaultIgnoresPrecompiledExpressionForImplicitContextVariants() { + SpelParserConfiguration config = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null); + SpelExpressionParser parser = new SpelExpressionParser(config); + // "order" resides in the public Ordered interface and is therefore compilable. + SpelExpression expression = parser.parseRaw("order"); + + // Compile the expression via StandardEvaluationContext. + StandardEvaluationContext standardContext = new StandardEvaluationContext(); + assertThat(standardContext.isCompilationSupported()).isTrue(); + OrderedComponent component = new OrderedComponent(); + IntStream.rangeClosed(1, 2).forEach(i -> + assertThat(expression.getValue(standardContext, component, Integer.class)).isEqualTo(42)); + assertIsCompiled(expression); + + // Switch to a SimpleEvaluationContext set as the default context — the precompiled + // expression should be ignored for all four implicit context getValue() variants. + SimpleEvaluationContext simpleContext = SimpleEvaluationContext.forReadOnlyDataBinding() + .withRootObject(component) + .build(); + assertThat(simpleContext.isCompilationSupported()).isFalse(); + expression.setEvaluationContext(simpleContext); + + // Record interpretedCount before the simpleContext evaluations. + // checkCompile() — which increments interpretedCount as its very first action — + // is only reachable from the interpreted path. If the compiled path were taken + // instead, interpretedCount would not change. + int interpretedCountBefore = getInterpretedCount(expression); + assertThat(expression.getValue()).isEqualTo(42); + assertThat(getInterpretedCount(expression)).isEqualTo(interpretedCountBefore + 1); + + interpretedCountBefore = getInterpretedCount(expression); + assertThat(expression.getValue(Integer.class)).isEqualTo(42); + assertThat(getInterpretedCount(expression)).isEqualTo(interpretedCountBefore + 1); + + interpretedCountBefore = getInterpretedCount(expression); + assertThat(expression.getValue(component)).isEqualTo(42); + assertThat(getInterpretedCount(expression)).isEqualTo(interpretedCountBefore + 1); + + interpretedCountBefore = getInterpretedCount(expression); + assertThat(expression.getValue(component, Integer.class)).isEqualTo(42); + assertThat(getInterpretedCount(expression)).isEqualTo(interpretedCountBefore + 1); + + // compiledAst is still set: the compiled expression was not cleared, rather merely ignored. + assertIsCompiled(expression); + } + @Test // gh-28043 void changingRegisteredVariableTypeDoesNotResultInFailureInMixedMode() { SpelParserConfiguration config = new SpelParserConfiguration(SpelCompilerMode.MIXED, null); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelExpressionTestUtils.java b/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelExpressionTestUtils.java index 124071379e3..2e55822f934 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelExpressionTestUtils.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelExpressionTestUtils.java @@ -17,6 +17,7 @@ package org.springframework.expression.spel.standard; import java.lang.reflect.Field; +import java.util.concurrent.atomic.AtomicInteger; import org.springframework.expression.Expression; @@ -26,6 +27,7 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests utilities for {@link SpelExpression}. * * @author Stephane Nicoll + * @author Sam Brannen */ public abstract class SpelExpressionTestUtils { @@ -41,4 +43,33 @@ public abstract class SpelExpressionTestUtils { } } + public static void assertIsNotCompiled(Expression expression) { + try { + Field field = SpelExpression.class.getDeclaredField("compiledAst"); + field.setAccessible(true); + Object object = field.get(expression); + assertThat(object).isNull(); + } + catch (Exception ex) { + throw new AssertionError(ex.getMessage(), ex); + } + } + + /** + * Return the current interpreted evaluation count for the given expression. + *

This counter is incremented exclusively by the interpreted evaluation path + * (inside {@code checkCompile()}), so it serves as a reliable witness for + * distinguishing interpreted from compiled evaluations in tests. + */ + public static int getInterpretedCount(Expression expression) { + try { + Field field = SpelExpression.class.getDeclaredField("interpretedCount"); + field.setAccessible(true); + return ((AtomicInteger) field.get(expression)).get(); + } + catch (Exception ex) { + throw new AssertionError(ex.getMessage(), ex); + } + } + } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/support/SimpleEvaluationContextTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/support/SimpleEvaluationContextTests.java index f3e5a2cfa3a..79f04df0c0f 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/support/SimpleEvaluationContextTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/support/SimpleEvaluationContextTests.java @@ -53,6 +53,24 @@ class SimpleEvaluationContextTests { private final Model model = new Model(); + @Test + void compilationNotSupportedByDefault() { + assertThat(SimpleEvaluationContext.forReadOnlyDataBinding().build().isCompilationSupported()).isFalse(); + assertThat(SimpleEvaluationContext.forReadWriteDataBinding().build().isCompilationSupported()).isFalse(); + assertThat(SimpleEvaluationContext.forPropertyAccessors(DataBindingPropertyAccessor.forReadOnlyAccess()) + .build().isCompilationSupported()).isFalse(); + } + + @Test + void compilationSupportedViaBuilder() { + assertThat(SimpleEvaluationContext.forReadOnlyDataBinding() + .withCompilationSupported().build().isCompilationSupported()).isTrue(); + assertThat(SimpleEvaluationContext.forReadWriteDataBinding() + .withCompilationSupported().build().isCompilationSupported()).isTrue(); + assertThat(SimpleEvaluationContext.forPropertyAccessors(DataBindingPropertyAccessor.forReadOnlyAccess()) + .withCompilationSupported().build().isCompilationSupported()).isTrue(); + } + @Test void forReadWriteDataBinding() { SimpleEvaluationContext context = SimpleEvaluationContext.forReadWriteDataBinding() diff --git a/spring-messaging/src/main/java/org/springframework/messaging/rsocket/annotation/support/MessagingRSocket.java b/spring-messaging/src/main/java/org/springframework/messaging/rsocket/annotation/support/MessagingRSocket.java index f2f1da621dd..19a6eb2f68b 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/rsocket/annotation/support/MessagingRSocket.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/rsocket/annotation/support/MessagingRSocket.java @@ -103,8 +103,9 @@ class MessagingRSocket implements RSocket { * @return completion handle for success or error */ public Mono handleConnectionSetupPayload(ConnectionSetupPayload payload) { - // frameDecoder does not apply to connectionSetupPayload - // so retain here since handle expects it. + // RSocket Java releases its own ConnectionSetupPayload reference + // For other frames, PayloadDecoder increases the refCount, and handles must releases it; + // retain here to match the release in handle (in retainDataAndReleasePayload) payload.retain(); return handle(payload, FrameType.SETUP); } @@ -187,32 +188,40 @@ class MessagingRSocket implements RSocket { private MessageHeaders createHeaders( Payload payload, FrameType frameType, @Nullable AtomicReference> responseRef) { - MessageHeaderAccessor headers = new MessageHeaderAccessor(); - headers.setLeaveMutable(true); + try { + MessageHeaderAccessor headers = new MessageHeaderAccessor(); + headers.setLeaveMutable(true); - Map metadataValues = this.metadataExtractor.extract(payload, this.metadataMimeType); + Map metadataValues = this.metadataExtractor.extract(payload, this.metadataMimeType); - metadataValues.putIfAbsent(MetadataExtractor.ROUTE_KEY, ""); - for (Map.Entry entry : metadataValues.entrySet()) { - if (entry.getKey().equals(MetadataExtractor.ROUTE_KEY)) { - RouteMatcher.Route route = this.routeMatcher.parseRoute((String) entry.getValue()); - headers.setHeader(DestinationPatternsMessageCondition.LOOKUP_DESTINATION_HEADER, route); + metadataValues.putIfAbsent(MetadataExtractor.ROUTE_KEY, ""); + for (Map.Entry entry : metadataValues.entrySet()) { + if (entry.getKey().equals(MetadataExtractor.ROUTE_KEY)) { + RouteMatcher.Route route = this.routeMatcher.parseRoute((String) entry.getValue()); + headers.setHeader(DestinationPatternsMessageCondition.LOOKUP_DESTINATION_HEADER, route); + } + else { + headers.setHeader(entry.getKey(), entry.getValue()); + } } - else { - headers.setHeader(entry.getKey(), entry.getValue()); + + headers.setContentType(this.dataMimeType); + headers.setHeader(RSocketFrameTypeMessageCondition.FRAME_TYPE_HEADER, frameType); + headers.setHeader(RSocketRequesterMethodArgumentResolver.RSOCKET_REQUESTER_HEADER, this.requester); + if (responseRef != null) { + headers.setHeader(RSocketPayloadReturnValueHandler.RESPONSE_HEADER, responseRef); } - } + headers.setHeader(HandlerMethodReturnValueHandler.DATA_BUFFER_FACTORY_HEADER, + this.strategies.dataBufferFactory()); - headers.setContentType(this.dataMimeType); - headers.setHeader(RSocketFrameTypeMessageCondition.FRAME_TYPE_HEADER, frameType); - headers.setHeader(RSocketRequesterMethodArgumentResolver.RSOCKET_REQUESTER_HEADER, this.requester); - if (responseRef != null) { - headers.setHeader(RSocketPayloadReturnValueHandler.RESPONSE_HEADER, responseRef); + return headers.getMessageHeaders(); + } + catch (Throwable ex) { + if (payload.refCnt() > 0) { + payload.release(); + } + throw ex; } - headers.setHeader(HandlerMethodReturnValueHandler.DATA_BUFFER_FACTORY_HEADER, - this.strategies.dataBufferFactory()); - - return headers.getMessageHeaders(); } } diff --git a/spring-web/src/main/java/org/springframework/http/ContentDisposition.java b/spring-web/src/main/java/org/springframework/http/ContentDisposition.java index 6e2e5f2eb1d..ababdaf4dc2 100644 --- a/spring-web/src/main/java/org/springframework/http/ContentDisposition.java +++ b/spring-web/src/main/java/org/springframework/http/ContentDisposition.java @@ -172,17 +172,17 @@ public final class ContentDisposition { sb.append(this.type); } if (this.name != null) { - sb.append("; name=\"").append(this.name).append('\"'); + sb.append("; name=\""); + appendName(sb, this.name).append('\"'); } if (this.filename != null) { if (this.charset == null || StandardCharsets.US_ASCII.equals(this.charset)) { - sb.append("; filename=\"") - .append(encodeQuotedPairs(this.filename)) - .append('\"'); + sb.append("; filename=\""); + appendName(sb, this.filename).append('\"'); } else { sb.append("; filename=\"") - .append(transliterateToAscii(encodeQuotedPairs(this.filename))) + .append(transliterateToAscii(appendName(new StringBuilder(), this.filename).toString())) .append("\"; filename*=") .append(encodeRfc5987Filename(this.filename, this.charset)); } @@ -253,7 +253,7 @@ public final class ContentDisposition { part.substring(eqIndex + 2, part.length() - 1) : part.substring(eqIndex + 1)); if (attribute.equals("name") ) { - name = value; + name = (value.indexOf('\\') != -1 ? decodeQuotedPairs(value) : value); } else if (attribute.equals("filename*") ) { int idx1 = value.indexOf('\''); @@ -503,19 +503,20 @@ public final class ContentDisposition { return sb.toString(); } - private static String encodeQuotedPairs(String filename) { - if (filename.indexOf('"') == -1 && filename.indexOf('\\') == -1) { - return filename; - } - StringBuilder sb = new StringBuilder(); - for (int i = 0; i < filename.length() ; i++) { - char c = filename.charAt(i); - if (c == '"' || c == '\\') { - sb.append('\\'); + private static StringBuilder appendName(StringBuilder buffer, String name) { + for (int i = 0; i < name.length() ; i++) { + char c = name.charAt(i); + // strip control characters + if (c <= 0x1F || c == 0x7F) { + continue; } - sb.append(c); + // encode quoted pairs + if (c == '"' || c == '\\') { + buffer.append('\\'); + } + buffer.append(c); } - return sb.toString(); + return buffer; } private static String decodeQuotedPairs(String filename) { diff --git a/spring-web/src/main/java/org/springframework/http/codec/ServerSentEvent.java b/spring-web/src/main/java/org/springframework/http/codec/ServerSentEvent.java index f2af79ab2ca..e77f3b8a08b 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/ServerSentEvent.java +++ b/spring-web/src/main/java/org/springframework/http/codec/ServerSentEvent.java @@ -20,8 +20,8 @@ import java.time.Duration; import org.jspecify.annotations.Nullable; -import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; +import org.springframework.web.util.SseUtils; /** * Representation for a Server-Sent Event for use with Spring's reactive Web support. @@ -112,7 +112,7 @@ public final class ServerSentEvent { } if (this.comment != null) { sb.append(':'); - appendEscaped(this.comment, "\n:", sb); + SseUtils.appendFieldValue("", this.comment, sb); sb.append('\n'); } if (this.data != null) { @@ -125,30 +125,6 @@ public final class ServerSentEvent { sb.append(fieldName).append(':').append(fieldValue).append('\n'); } - private void appendEscaped(String input, String replacement, StringBuilder sb) { - if (input.indexOf('\n') == -1 && input.indexOf('\r') == -1) { - sb.append(input); - } - else { - int length = input.length(); - for (int i = 0; i < length; i++) { - char c = input.charAt(i); - if (c == '\r') { - if (i + 1 < length && input.charAt(i + 1) == '\n') { - i++; - } - sb.append(replacement); - } - else if (c == '\n') { - sb.append(replacement); - } - else { - sb.append(c); - } - } - } - } - @Override public boolean equals(@Nullable Object other) { return (this == other || (other instanceof ServerSentEvent that && @@ -265,23 +241,18 @@ public final class ServerSentEvent { @Override public Builder id(String id) { - checkEvent(id); + SseUtils.assertNoLineSeparator(id); this.id = id; return this; } @Override public Builder event(String event) { - checkEvent(event); + SseUtils.assertNoLineSeparator(event); this.event = event; return this; } - private static void checkEvent(String content) { - Assert.isTrue(content.indexOf('\n') == -1 && content.indexOf('\r') == -1, - "illegal character '\\n' or '\\r' in event content"); - } - @Override public Builder retry(Duration retry) { this.retry = retry; diff --git a/spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageWriter.java index 24b6e46207b..8e7f02caaa4 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageWriter.java @@ -40,6 +40,7 @@ import org.springframework.http.ReactiveHttpOutputMessage; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.util.Assert; +import org.springframework.web.util.SseUtils; /** * {@code HttpMessageWriter} for {@code "text/event-stream"} responses. @@ -142,27 +143,7 @@ public class ServerSentEventHttpMessageWriter implements HttpMessageWriter * */ - public static Flux> split(Flux xmlEventFlux, Set names) { - return xmlEventFlux.handle(new SplitHandler(names)); + public static Flux> split( + Flux xmlEventFlux, Set names, XmlEventDecoder.@Nullable ReceivedByteTracker byteTracker) { + + return xmlEventFlux.handle(new SplitHandler(names, byteTracker)); } @@ -150,14 +153,17 @@ abstract class Jaxb2Helper { private final Set names; + private final XmlEventDecoder.ReceivedByteTracker byteTracker; + private @Nullable List events; private int elementDepth = 0; private int barrier = Integer.MAX_VALUE; - public SplitHandler(Set names) { + public SplitHandler(Set names, XmlEventDecoder.@Nullable ReceivedByteTracker byteTracker) { this.names = names; + this.byteTracker = (byteTracker != null ? byteTracker : XmlEventDecoder.ReceivedByteTracker.NO_OP); } @Override @@ -179,11 +185,19 @@ abstract class Jaxb2Helper { if (event.isEndElement()) { this.elementDepth--; if (this.elementDepth == this.barrier) { - this.barrier = Integer.MAX_VALUE; Assert.state(this.events != null, "No XMLEvent List"); sink.next(this.events); + this.barrier = Integer.MAX_VALUE; + this.events = null; } } + if (this.events == null) { + this.byteTracker.reset(); + } + else if (this.byteTracker.isMaxInMemorySizeExceeded()) { + throw new DataBufferLimitException( + "Exceeded limit on max bytes per XML node: " + this.byteTracker.getMaxInMemorySize()); + } } } diff --git a/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java b/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java index c7721fd9b81..1dac0277c02 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java @@ -145,12 +145,16 @@ public class Jaxb2XmlDecoder extends AbstractDecoder { public Flux decode(Publisher inputStream, ResolvableType elementType, @Nullable MimeType mimeType, @Nullable Map hints) { + XmlEventDecoder.ReceivedByteTracker byteTracker = + new XmlEventDecoder.ReceivedByteTracker(this.maxInMemorySize); + Flux xmlEventFlux = this.xmlEventDecoder.decode( - inputStream, ResolvableType.forClass(XMLEvent.class), mimeType, hints); + inputStream, ResolvableType.forClass(XMLEvent.class), mimeType, + Hints.merge(hints, XmlEventDecoder.BYTE_TRACKER_HINT, byteTracker)); Class outputClass = elementType.toClass(); Set typeNames = Jaxb2Helper.toQNames(outputClass); - Flux> splitEvents = Jaxb2Helper.split(xmlEventFlux, typeNames); + Flux> splitEvents = Jaxb2Helper.split(xmlEventFlux, typeNames, byteTracker); return splitEvents.map(events -> { Object value = unmarshal(events, outputClass); diff --git a/spring-web/src/main/java/org/springframework/http/codec/xml/XmlEventDecoder.java b/spring-web/src/main/java/org/springframework/http/codec/xml/XmlEventDecoder.java index d766b3ee4e4..8ded46a9c3c 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/xml/XmlEventDecoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/xml/XmlEventDecoder.java @@ -84,6 +84,12 @@ import org.springframework.util.xml.StaxUtils; */ public class XmlEventDecoder extends AbstractDecoder { + /** + * Hint key for a {@link ReceivedByteTracker} instance that callers can use + * to track the number of received bytes. + */ + public static final String BYTE_TRACKER_HINT = XmlEventDecoder.class.getName() + ".byteTracker"; + private static final XMLInputFactory inputFactory = StaxUtils.createDefensiveInputFactory(); private static final boolean AALTO_PRESENT = ClassUtils.isPresent( @@ -100,10 +106,13 @@ public class XmlEventDecoder extends AbstractDecoder { /** - * Set the max number of bytes that can be buffered by this decoder. This - * is either the size the entire input when decoding as a whole, or when - * using async parsing via Aalto XML, it is size one top-level XML tree. - * When the limit is exceeded, {@link DataBufferLimitException} is raised. + * Set the max number of bytes this decoder should buffer in memory resulting + * in a {@link DataBufferLimitException} when the limit is exceeded. + *

When joining all buffers and decoding as a whole, the limit is applied + * to the entire input. + *

>When using Aalto XML async parsing, the limit does not apply at the + * level of this decoder because the XML events parsed from each buffer are + * emitted immediately and the buffer is released. *

By default this is set to 256K. * @param byteCount the max number of bytes to buffer, or -1 for unlimited * @since 5.1.11 @@ -126,7 +135,7 @@ public class XmlEventDecoder extends AbstractDecoder { @Nullable MimeType mimeType, @Nullable Map hints) { if (this.useAalto) { - AaltoDataBufferToXmlEvent mapper = new AaltoDataBufferToXmlEvent(this.maxInMemorySize); + AaltoDataBufferToXmlEvent mapper = new AaltoDataBufferToXmlEvent(hints); return Flux.from(input) .flatMapIterable(mapper) .doFinally(signalType -> mapper.endOfInput()); @@ -155,7 +164,7 @@ public class XmlEventDecoder extends AbstractDecoder { /* * Separate static class to isolate Aalto dependency. */ - private static class AaltoDataBufferToXmlEvent implements Function> { + private static final class AaltoDataBufferToXmlEvent implements Function> { private static final AsyncXMLInputFactory inputFactory = StaxUtils.createDefensiveInputFactory(InputFactoryImpl::new); @@ -165,22 +174,19 @@ public class XmlEventDecoder extends AbstractDecoder { private final XMLEventAllocator eventAllocator = EventAllocatorImpl.getDefaultInstance(); - private final int maxInMemorySize; + @Nullable + private final ReceivedByteTracker byteTracker; - private int byteCount; - - private int elementDepth; - - - public AaltoDataBufferToXmlEvent(int maxInMemorySize) { - this.maxInMemorySize = maxInMemorySize; + private AaltoDataBufferToXmlEvent(@Nullable Map hints) { + this.byteTracker = (hints != null ? (ReceivedByteTracker) hints.get(BYTE_TRACKER_HINT) : null); } - @Override public List apply(DataBuffer dataBuffer) { try { - increaseByteCount(dataBuffer); + if (this.byteTracker != null) { + this.byteTracker.incrementByteCount(dataBuffer); + } AsyncByteBufferFeeder inputFeeder = this.streamReader.getInputFeeder(); try (DataBuffer.ByteBufferIterator iterator = dataBuffer.readableByteBuffers()) { while (iterator.hasNext()) { @@ -199,12 +205,8 @@ public class XmlEventDecoder extends AbstractDecoder { if (event.isEndDocument()) { break; } - checkDepthAndResetByteCount(event); } } - if (this.maxInMemorySize > 0 && this.byteCount > this.maxInMemorySize) { - raiseLimitException(); - } return events; } catch (XMLStreamException ex) { @@ -215,40 +217,55 @@ public class XmlEventDecoder extends AbstractDecoder { } } - private void increaseByteCount(DataBuffer dataBuffer) { - if (this.maxInMemorySize > 0) { - if (dataBuffer.readableByteCount() > Integer.MAX_VALUE - this.byteCount) { - raiseLimitException(); - } - else { - this.byteCount += dataBuffer.readableByteCount(); - } - } - } - - private void checkDepthAndResetByteCount(XMLEvent event) { - if (this.maxInMemorySize > 0) { - if (event.isStartElement()) { - this.byteCount = this.elementDepth == 1 ? 0 : this.byteCount; - this.elementDepth++; - } - else if (event.isEndElement()) { - this.elementDepth--; - this.byteCount = this.elementDepth == 1 ? 0 : this.byteCount; - } - } - } - - private void raiseLimitException() { - throw new DataBufferLimitException( - "Exceeded limit on max bytes per XML top-level node: " + this.maxInMemorySize); - } - public void endOfInput() { this.streamReader.getInputFeeder().endOfInput(); } } + /** + * Callers of {@link XmlEventDecoder} that buffer emitted XML events at a + * higher level, can pass an instance of this tracker as an + * {@link XmlEventDecoder#BYTE_TRACKER_HINT} to monitor the total number of + * bytes received, and to reset periodically. + *

For use with Aalto XML async parsing only, in which case this decoder + * parses releases each buffer immediately. + */ + public static class ReceivedByteTracker { + + /** An instance to use when there is no limit. */ + public static final ReceivedByteTracker NO_OP = new ReceivedByteTracker(-1); + + private final int maxInMemorySize; + + private int byteCount; + + public ReceivedByteTracker(int maxInMemorySize) { + this.maxInMemorySize = maxInMemorySize; + } + + public int getMaxInMemorySize() { + return this.maxInMemorySize; + } + + public boolean isMaxInMemorySizeExceeded() { + return (this.maxInMemorySize != -1 && this.byteCount > this.maxInMemorySize); + } + + public void reset() { + this.byteCount = 0; + } + + private void incrementByteCount(DataBuffer buffer) { + if (this.maxInMemorySize != -1) { + this.byteCount += buffer.readableByteCount(); + } + } + + @Override + public String toString() { + return this.byteCount + " bytes"; + } + } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpResponse.java index 9ad748a85c3..c9ee8eeb563 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/JettyCoreServerHttpResponse.java @@ -212,9 +212,8 @@ class JettyCoreServerHttpResponse extends AbstractServerHttpResponse implements } @Override - public @Nullable SameSite getSameSite() { - // Adding non-null return site breaks tests. - return null; + public SameSite getSameSite() { + return SameSite.from(this.responseCookie.getSameSite()); } @Override diff --git a/spring-web/src/main/java/org/springframework/web/bind/EscapedErrors.java b/spring-web/src/main/java/org/springframework/web/bind/EscapedErrors.java index 65723509dc3..cb711b7236e 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/EscapedErrors.java +++ b/spring-web/src/main/java/org/springframework/web/bind/EscapedErrors.java @@ -170,12 +170,12 @@ public class EscapedErrors implements Errors { @Override public List getFieldErrors() { - return this.source.getFieldErrors(); + return escapeObjectErrors(this.source.getFieldErrors()); } @Override public @Nullable FieldError getFieldError() { - return this.source.getFieldError(); + return escapeObjectError(this.source.getFieldError()); } @Override diff --git a/spring-web/src/main/java/org/springframework/web/filter/UrlHandlerFilter.java b/spring-web/src/main/java/org/springframework/web/filter/UrlHandlerFilter.java index c3e99de04b7..18759fe694c 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/UrlHandlerFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/UrlHandlerFilter.java @@ -308,6 +308,9 @@ public final class UrlHandlerFilter extends OncePerRequestFilter { throws IOException { String location = trimTrailingSlash(request.getRequestURI()); + if (location.length() > 2 && location.startsWith("//")) { + location = (location.charAt(2) != '/' ? location.substring(1) : location); + } if (StringUtils.hasText(request.getQueryString())) { location += "?" + request.getQueryString(); } diff --git a/spring-web/src/main/java/org/springframework/web/filter/reactive/UrlHandlerFilter.java b/spring-web/src/main/java/org/springframework/web/filter/reactive/UrlHandlerFilter.java index 4afe02cdd64..057bd7bb188 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/reactive/UrlHandlerFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/reactive/UrlHandlerFilter.java @@ -299,12 +299,14 @@ public final class UrlHandlerFilter implements WebFilter { @Override public Mono handleInternal(ServerWebExchange exchange, WebFilterChain chain) { ServerHttpRequest request = exchange.getRequest(); - String query = request.getURI().getRawQuery(); String location = trimTrailingSlash(request); + if (location.length() > 2 && location.startsWith("//")) { + location = (location.charAt(2) != '/' ? location.substring(1) : location); + } + String query = request.getURI().getRawQuery(); if (StringUtils.hasText(query)) { location += "?" + query; } - ServerHttpResponse response = exchange.getResponse(); response.setStatusCode(this.statusCode); response.getHeaders().set(HttpHeaders.LOCATION, location); diff --git a/spring-web/src/main/java/org/springframework/web/util/SseUtils.java b/spring-web/src/main/java/org/springframework/web/util/SseUtils.java new file mode 100644 index 00000000000..ceead086ab7 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/web/util/SseUtils.java @@ -0,0 +1,77 @@ +/* + * Copyright 2002-present 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.util; + +import org.springframework.util.Assert; + +/** + * Utility methods for writing content as + * Server-Sent Events, + * shared by the Servlet and Reactive SSE support. + * + * @author Brian Clozel + * @since 7.0.9 + */ +public abstract class SseUtils { + + /** + * Append {@code value} to {@code output}, replacing each line separator + * ({@code "\n"}, {@code "\r"}, or {@code "\r\n"}) it contains with a new + * {@code field} line (that is, {@code "\n" + field + ":"}). This keeps a + * multi-line field value from breaking out of the current SSE field when + * written on the wire. + * @param field the name of the SSE field that {@code value} belongs to + * (for example, {@code "data"}), or an empty string for a comment + * @param value the field value to escape and append + * @param output the {@code StringBuilder} to append the escaped value to + */ + public static void appendFieldValue(String field, String value, StringBuilder output) { + if (value.indexOf('\n') == -1 && value.indexOf('\r') == -1) { + output.append(value); + return; + } + String lineSeparatorReplacement = "\n" + field + ":"; + int length = value.length(); + for (int i = 0; i < length; i++) { + char c = value.charAt(i); + if (c == '\r') { + if (i + 1 < length && value.charAt(i + 1) == '\n') { + i++; + } + output.append(lineSeparatorReplacement); + } + else if (c == '\n') { + output.append(lineSeparatorReplacement); + } + else { + output.append(c); + } + } + } + + /** + * Assert that the given single-line SSE field value, such as an + * {@code id} or {@code event} name, does not contain a line separator. + * @param content the field value to check + * @throws IllegalArgumentException if {@code content} contains {@code "\n"} or {@code "\r"} + */ + public static void assertNoLineSeparator(String content) { + Assert.isTrue(content.indexOf('\n') == -1 && content.indexOf('\r') == -1, + "illegal character '\\n' or '\\r' in event content"); + } + +} diff --git a/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java b/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java index 2928b836763..6b3f40ea11a 100644 --- a/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java +++ b/spring-web/src/test/java/org/springframework/http/ContentDispositionTests.java @@ -181,6 +181,15 @@ class ContentDispositionTests { assertThat(cd.getFilename()).isEqualTo("foo\\bar \"baz\" qux \\\" quux.txt"); } + @Test + void parseBackslashInName() { + String s = "form-data; name=\"foo\\\"bar\"; filename=\"foo.txt\""; + ContentDisposition cd = ContentDisposition.parse(s); + assertThat(cd.getName()).isEqualTo("foo\"bar"); + assertThat(cd.getFilename()).isEqualTo("foo.txt"); + assertThat(cd.toString()).isEqualTo(s); + } + @Test void parseBackslashInLastPosition() { ContentDisposition cd = ContentDisposition.parse("form-data; name=\"foo\"; filename=\"bar\\\""); diff --git a/spring-web/src/test/java/org/springframework/http/codec/multipart/PartEventHttpMessageReaderTests.java b/spring-web/src/test/java/org/springframework/http/codec/multipart/PartEventHttpMessageReaderTests.java index 79d7881a254..5f9671b91e2 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/multipart/PartEventHttpMessageReaderTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/multipart/PartEventHttpMessageReaderTests.java @@ -255,6 +255,21 @@ class PartEventHttpMessageReaderTests extends AbstractLeakCheckingTests { .verify(); } + @Test + void partSizeTooLargeWithUnlimitedMemorySize() { + MockServerHttpRequest request = createRequest("simple.multipart", "simple-boundary"); + + PartEventHttpMessageReader reader = new PartEventHttpMessageReader(); + reader.setMaxPartSize(10); + reader.setMaxInMemorySize(-1); + + Flux result = reader.read(forClass(PartEvent.class), request, emptyMap()); + + StepVerifier.create(result) + .expectError(DataBufferLimitException.class) + .verify(); + } + @Test void formPartTooLarge() { MockServerHttpRequest request = createRequest( diff --git a/spring-web/src/test/java/org/springframework/http/codec/xml/Jaxb2XmlDecoderTests.java b/spring-web/src/test/java/org/springframework/http/codec/xml/Jaxb2XmlDecoderTests.java index 8df28da8b96..108c2e113fd 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/xml/Jaxb2XmlDecoderTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/xml/Jaxb2XmlDecoderTests.java @@ -35,7 +35,9 @@ import reactor.test.StepVerifier; import org.springframework.core.ResolvableType; import org.springframework.core.codec.DecodingException; +import org.springframework.core.codec.Hints; import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DataBufferLimitException; import org.springframework.core.testfixture.io.buffer.AbstractLeakCheckingTests; import org.springframework.http.MediaType; import org.springframework.util.MimeType; @@ -92,7 +94,7 @@ class Jaxb2XmlDecoderTests extends AbstractLeakCheckingTests { @Test void splitOneBranches() { Flux xmlEvents = this.xmlEventDecoder.decode(toDataBufferMono(POJO_ROOT), null, null, HINTS); - Flux> result = Jaxb2Helper.split(xmlEvents, Set.of(new QName("pojo"))); + Flux> result = Jaxb2Helper.split(xmlEvents, Set.of(new QName("pojo")), null); StepVerifier.create(result) .consumeNextWith(events -> { @@ -113,8 +115,7 @@ class Jaxb2XmlDecoderTests extends AbstractLeakCheckingTests { @Test void splitMultipleBranches() { Flux xmlEvents = this.xmlEventDecoder.decode(toDataBufferMono(POJO_CHILD), null, null, HINTS); - Flux> result = Jaxb2Helper.split(xmlEvents, Set.of(new QName("pojo"))); - + Flux> result = Jaxb2Helper.split(xmlEvents, Set.of(new QName("pojo")), null); StepVerifier.create(result) .consumeNextWith(events -> { @@ -143,6 +144,34 @@ class Jaxb2XmlDecoderTests extends AbstractLeakCheckingTests { .verify(); } + @Test + void splitMultipleBranchesLimitExceeded() { + + Flux source = Flux.just( + "", + "foo", + "foofoo", + "barbar", + ""); + + XmlEventDecoder.ReceivedByteTracker byteTracker = new XmlEventDecoder.ReceivedByteTracker(30); + Map hints = Hints.from(XmlEventDecoder.BYTE_TRACKER_HINT, byteTracker); + Flux xmlEvents = this.xmlEventDecoder.decode(source.map(this::toToDataBuffer), null, null, hints); + Flux> result = Jaxb2Helper.split(xmlEvents, Set.of(new QName("pojo")), byteTracker); + + StepVerifier.create(result) + .consumeNextWith(events -> { + assertThat(events).hasSize(5); + assertStartElement(events.get(0), "pojo"); + assertStartElement(events.get(1), "foo"); + assertCharacters(events.get(2), "foo"); + assertEndElement(events.get(3), "foo"); + assertEndElement(events.get(4), "pojo"); + }) + .expectError(DataBufferLimitException.class) + .verify(); + } + private static void assertStartElement(XMLEvent event, String expectedLocalName) { assertThat(event.isStartElement()).isTrue(); assertThat(event.asStartElement().getName().getLocalPart()).isEqualTo(expectedLocalName); @@ -263,13 +292,18 @@ class Jaxb2XmlDecoderTests extends AbstractLeakCheckingTests { private Mono toDataBufferMono(String value) { return Mono.defer(() -> { - byte[] bytes = value.getBytes(StandardCharsets.UTF_8); - DataBuffer buffer = this.bufferFactory.allocateBuffer(bytes.length); - buffer.write(bytes); + DataBuffer buffer = toToDataBuffer(value); return Mono.just(buffer); }); } + private DataBuffer toToDataBuffer(String value) { + byte[] bytes = value.getBytes(StandardCharsets.UTF_8); + DataBuffer buffer = this.bufferFactory.allocateBuffer(bytes.length); + buffer.write(bytes); + return buffer; + } + @jakarta.xml.bind.annotation.XmlType @XmlSeeAlso(Child.class) public abstract static class Parent { diff --git a/spring-web/src/test/java/org/springframework/http/codec/xml/XmlEventDecoderTests.java b/spring-web/src/test/java/org/springframework/http/codec/xml/XmlEventDecoderTests.java index 36425c1198c..7a97bf30f23 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/xml/XmlEventDecoderTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/xml/XmlEventDecoderTests.java @@ -27,7 +27,6 @@ import reactor.core.publisher.Mono; import reactor.test.StepVerifier; import org.springframework.core.io.buffer.DataBuffer; -import org.springframework.core.io.buffer.DataBufferLimitException; import org.springframework.core.testfixture.io.buffer.AbstractLeakCheckingTests; import static org.assertj.core.api.Assertions.assertThat; @@ -88,28 +87,6 @@ class XmlEventDecoderTests extends AbstractLeakCheckingTests { .verify(); } - @Test - void toXMLEventsWithLimit() { - - this.decoder.setMaxInMemorySize(6); - - Flux source = Flux.just( - "", "", "foofoo", "", "", "barbarbar", "", ""); - - Flux events = this.decoder.decode( - source.map(this::stringBuffer), null, null, Collections.emptyMap()); - - StepVerifier.create(events) - .consumeNextWith(e -> assertThat(e.isStartDocument()).isTrue()) - .consumeNextWith(e -> assertStartElement(e, "pojo")) - .consumeNextWith(e -> assertStartElement(e, "foo")) - .consumeNextWith(e -> assertCharacters(e, "foofoo")) - .consumeNextWith(e -> assertEndElement(e, "foo")) - .consumeNextWith(e -> assertStartElement(e, "bar")) - .expectError(DataBufferLimitException.class) - .verify(); - } - @Test void decodeErrorAalto() { Flux source = Flux.concat( diff --git a/spring-web/src/test/java/org/springframework/web/bind/EscapedErrorsTests.java b/spring-web/src/test/java/org/springframework/web/bind/EscapedErrorsTests.java index 96def23148a..1806e07c8ff 100644 --- a/spring-web/src/test/java/org/springframework/web/bind/EscapedErrorsTests.java +++ b/spring-web/src/test/java/org/springframework/web/bind/EscapedErrorsTests.java @@ -94,4 +94,25 @@ class EscapedErrorsTests { assertThat(ageError2.getCode()).as("Age error 2 code not escaped").isEqualTo("AGE_NOT_32 "); } + @Test + void noArgFieldErrorAccessorsEscapeRejectedValueAndMessage() { + TestBean tb = new TestBean(); + tb.setName(""); + + Errors errors = new EscapedErrors(new BindException(tb, "tb")); + errors.rejectValue("name", "NAME_INVALID", null, "message: "); + + FieldError fieldError = errors.getFieldError(); + assertThat(fieldError.getDefaultMessage()).as("No-arg getFieldError() message escaped") + .isEqualTo("message: <tag>"); + assertThat(fieldError.getRejectedValue()).as("No-arg getFieldError() rejected value escaped") + .isEqualTo("<script>alert(1)</script>"); + + FieldError fieldErrorInList = errors.getFieldErrors().get(0); + assertThat(fieldErrorInList.getDefaultMessage()).as("No-arg getFieldErrors() message escaped") + .isEqualTo("message: <tag>"); + assertThat(fieldErrorInList.getRejectedValue()).as("No-arg getFieldErrors() rejected value escaped") + .isEqualTo("<script>alert(1)</script>"); + } + } diff --git a/spring-web/src/test/java/org/springframework/web/filter/UrlHandlerFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/UrlHandlerFilterTests.java index d8ed4ba81d9..07696b0944a 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/UrlHandlerFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/UrlHandlerFilterTests.java @@ -74,11 +74,16 @@ class UrlHandlerFilterTests { @Test void redirect() throws Exception { - HttpStatus status = HttpStatus.PERMANENT_REDIRECT; - UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/*").redirect(status).build(); + testRedirect("/**", "/path/123/", "/path/123"); + testRedirect("/**", "//path/123/", "/path/123"); + testRedirect("/**", "///path/123/", "///path/123"); + } - String path = "/path/123"; - MockHttpServletRequest request = new MockHttpServletRequest("GET", path + "/"); + private void testRedirect(String pattern, String path, String location) throws Exception { + HttpStatus status = HttpStatus.PERMANENT_REDIRECT; + UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler(pattern).redirect(status).build(); + + MockHttpServletRequest request = new MockHttpServletRequest("GET", path); MockHttpServletResponse response = new MockHttpServletResponse(); MockFilterChain chain = new MockFilterChain(); @@ -89,7 +94,7 @@ class UrlHandlerFilterTests { assertThat(chain.getRequest()).isNull(); assertThat(response.getStatus()).isEqualTo(status.value()); - assertThat(response.getHeader(HttpHeaders.LOCATION)).isEqualTo(path + "?" + queryString); + assertThat(response.getHeader(HttpHeaders.LOCATION)).isEqualTo(location + "?" + queryString); assertThat(response.isCommitted()).isTrue(); } diff --git a/spring-web/src/test/java/org/springframework/web/filter/reactive/UrlHandlerFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/reactive/UrlHandlerFilterTests.java index c3cc1e65554..32cef22215f 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/reactive/UrlHandlerFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/reactive/UrlHandlerFilterTests.java @@ -17,12 +17,14 @@ package org.springframework.web.filter.reactive; import java.net.URI; +import java.net.URISyntaxException; import java.util.List; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import reactor.core.publisher.Mono; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.web.server.ServerWebExchange; @@ -59,20 +61,23 @@ class UrlHandlerFilterTests { } @Test - void redirect() { - HttpStatus status = HttpStatus.PERMANENT_REDIRECT; - UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/*").redirect(status).build(); + void redirect() throws URISyntaxException { + testRedirect("/**", new URI(null, null, "/path/123/", "foo=bar", null), "/path/123?foo=bar"); + // no way to create java.net.URI with leading slashes + } - String path = "/path/123"; - String queryString = "foo=bar"; - MockServerHttpRequest original = MockServerHttpRequest.get(path + "/?" + queryString).build(); - ServerWebExchange exchange = MockServerWebExchange.from(original); + private static void testRedirect(String pattern, URI uri, String location) { + HttpStatus status = HttpStatus.PERMANENT_REDIRECT; + UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler(pattern).redirect(status).build(); + + MockServerHttpRequest request = MockServerHttpRequest.method(HttpMethod.GET, uri).build(); + ServerWebExchange exchange = MockServerWebExchange.from(request); assertThatThrownBy(() -> invokeFilter(filter, exchange)) .hasMessageContaining("No argument value was captured"); assertThat(exchange.getResponse().getStatusCode()).isEqualTo(status); - assertThat(exchange.getResponse().getHeaders().getLocation()).isEqualTo(URI.create(path + "?" + queryString)); + assertThat(exchange.getResponse().getHeaders().getLocation()).isEqualTo(URI.create(location)); } @Test diff --git a/spring-web/src/test/java/org/springframework/web/util/SseUtilsTests.java b/spring-web/src/test/java/org/springframework/web/util/SseUtilsTests.java new file mode 100644 index 00000000000..94a649a0e2d --- /dev/null +++ b/spring-web/src/test/java/org/springframework/web/util/SseUtilsTests.java @@ -0,0 +1,81 @@ +/* + * Copyright 2002-present 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.util; + +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; + +/** + * Tests for {@link SseUtils}. + * @author Brian Clozel + */ +class SseUtilsTests { + + @Test + void appendFieldValueWithoutLineSeparatorAppendsAsIs() { + StringBuilder sb = new StringBuilder(); + sb.append("data:"); + SseUtils.appendFieldValue("data", "no newlines here", sb); + assertThat(sb).hasToString("data:no newlines here"); + } + + @ParameterizedTest(name = "{1}") + @MethodSource("newLineCharacters") + void appendFieldValueReplacesLineSeparatorWithFieldPrefix(String newLine, String description) { + StringBuilder sb = new StringBuilder(); + sb.append("data:"); + SseUtils.appendFieldValue("data", "first" + newLine + "second", sb); + assertThat(sb).hasToString("data:first\ndata:second"); + } + + @ParameterizedTest(name = "{1}") + @MethodSource("newLineCharacters") + void appendFieldValueUsesEmptyFieldForComments(String newLine, String description) { + StringBuilder sb = new StringBuilder(); + sb.append(":"); + SseUtils.appendFieldValue("", "first" + newLine + "second", sb); + assertThat(sb).hasToString(":first\n:second"); + } + + @Test + void assertNoLineSeparatorAcceptsPlainContent() { + SseUtils.assertNoLineSeparator("no newlines here"); + } + + @ParameterizedTest(name = "{1}") + @MethodSource("newLineCharacters") + void assertNoLineSeparatorRejectsLineSeparator(String newLine, String description) { + assertThatIllegalArgumentException().isThrownBy(() -> + SseUtils.assertNoLineSeparator("first" + newLine + "second")); + } + + static Stream newLineCharacters() { + return Stream.of( + Arguments.of("\n", "LF"), + Arguments.of("\r", "CR"), + Arguments.of("\r\n", "CRLF") + ); + } + +} diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RouterFunctions.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RouterFunctions.java index e1d469a6419..d6faabedce3 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RouterFunctions.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/RouterFunctions.java @@ -38,6 +38,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.codec.HttpMessageWriter; import org.springframework.http.server.reactive.HttpHandler; import org.springframework.util.Assert; +import org.springframework.web.cors.reactive.CorsUtils; import org.springframework.web.reactive.result.view.ViewResolver; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; @@ -1475,9 +1476,14 @@ public abstract class RouterFunctions { addAttributes(exchange, request); return this.routerFunction.route(request) .switchIfEmpty(createNotFoundError()) - .flatMap(handlerFunction -> wrapException(() -> handlerFunction.handle(request))) - .flatMap(response -> wrapException(() -> response.writeTo(exchange, - new HandlerStrategiesResponseContext(this.strategies)))); + .flatMap(handlerFunction -> wrapException(() -> { + if (CorsUtils.isPreFlightRequest(exchange.getRequest())) { + return handlePreFlightRequest(); + } + return handlerFunction.handle(request); + })) + .flatMap(response -> wrapException(() -> + response.writeTo(exchange, new HandlerStrategiesResponseContext(this.strategies)))); }); } @@ -1498,5 +1504,11 @@ public abstract class RouterFunctions { return Mono.error(ex); } } + + @SuppressWarnings("unchecked") + private static Mono handlePreFlightRequest() { + return (Mono) ServerResponse.status(HttpStatus.FORBIDDEN).build(); + } } + } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java index 5147d2c59c6..b0a7c0e41df 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java @@ -64,6 +64,7 @@ import org.springframework.web.reactive.result.HandlerResultHandlerSupport; import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.util.SseUtils; /** * {@code HandlerResultHandler} that encapsulates the view resolution algorithm @@ -603,8 +604,9 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport imp finally { DataBufferUtils.release(buffer); } - text = text.replace("\n", "\ndata:"); - return bufferFactory.wrap(text.getBytes(charset)); + StringBuilder escaped = new StringBuilder(); + SseUtils.appendFieldValue("data", text, escaped); + return bufferFactory.wrap(escaped.toString().getBytes(charset)); }); return Flux.concat(Flux.just(prefix), content, Flux.just(suffix)); @@ -614,6 +616,7 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport imp byte[] bytes = text.getBytes(charset); return bufferFactory.wrap(bytes); } + } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.java b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.java index 9eed2bf0030..e29cb2279e4 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.java @@ -195,12 +195,12 @@ public class HandshakeWebSocketService implements WebSocketService, Lifecycle { if (HttpMethod.GET == method) { if (!"WebSocket".equalsIgnoreCase(headers.getUpgrade())) { - return handleBadRequest(exchange, "Invalid 'Upgrade' header: " + headers); + return handleBadRequest(exchange, "Can \"Upgrade\" only to \"WebSocket\"."); } List connectionValue = headers.getConnection(); if (!connectionValue.contains("Upgrade") && !connectionValue.contains("upgrade")) { - return handleBadRequest(exchange, "Invalid 'Connection' header: " + headers); + return handleBadRequest(exchange, "\"Connection\" must be \"upgrade\"."); } String key = headers.getFirst(SEC_WEBSOCKET_KEY); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RouterFunctionsTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RouterFunctionsTests.java index af891328cf3..5d08a460089 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RouterFunctionsTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/server/RouterFunctionsTests.java @@ -28,11 +28,14 @@ import reactor.test.StepVerifier; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseCookie; import org.springframework.http.server.reactive.HttpHandler; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.web.cors.CorsConfiguration; +import org.springframework.web.cors.reactive.CorsWebFilter; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilter; @@ -282,6 +285,58 @@ class RouterFunctionsTests { assertThat(httpResponse.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); } + @Test + void toHttpHandlerPreFlightRequestDefaultHandling() { + RouterFunction routerFunction = + RouterFunctions.route(RequestPredicates.all(), request -> ServerResponse.accepted().build()); + + HttpHandler handler = RouterFunctions.toHttpHandler(routerFunction); + assertThat(handler).isNotNull(); + + MockServerHttpRequest httpRequest = MockServerHttpRequest.options("https://localhost") + .header("Origin", "https://example.com") + .header(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "PUT") + .build(); + + MockServerHttpResponse httpResponse = new MockServerHttpResponse(); + handler.handle(httpRequest, httpResponse).block(); + + assertThat(httpResponse.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN); + } + + @Test + void toHttpHandlerPreFlightRequestHandled() { + + CorsWebFilter corsFilter = new CorsWebFilter(exchange -> { + if (exchange.getRequest().getPath().value().equals("/path")) { + CorsConfiguration corsConfig = new CorsConfiguration(); + corsConfig.addAllowedOrigin("https://example.com"); + corsConfig.addAllowedMethod(HttpMethod.PUT); + return corsConfig; + } + return null; + }); + + RouterFunction routerFunction = RouterFunctions.route() + .PUT("/path", request -> { + throw new IllegalStateException("Not expected"); + }) + .build(); + + HttpHandler handler = RouterFunctions.toHttpHandler( + routerFunction, HandlerStrategies.builder().webFilter(corsFilter).build()); + + MockServerHttpRequest httpRequest = MockServerHttpRequest.options("https://localhost/path") + .header("Origin", "https://example.com") + .header(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "PUT") + .build(); + + MockServerHttpResponse httpResponse = new MockServerHttpResponse(); + handler.handle(httpRequest, httpResponse).block(); + + assertThat(httpResponse.getStatusCode()).isNull(); + } + @Test void toHttpHandlerWebFilter() { AtomicBoolean filterInvoked = new AtomicBoolean(); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/FragmentViewResolutionResultHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/FragmentViewResolutionResultHandlerTests.java index 7a9ff80abbf..db95230fc13 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/FragmentViewResolutionResultHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/FragmentViewResolutionResultHandlerTests.java @@ -119,6 +119,21 @@ class FragmentViewResolutionResultHandlerTests { """); } + @Test + void escapeViewFragment() { + Fragment fragment = Fragment.create("fragment1", Map.of("foo", "Foo\n and Bar")); + testSse(Flux.just(fragment), + on(Handler.class).resolveReturnType(Flux.class, Fragment.class), + """ + event:fragment1 + data:

+ data: Hello Foo + data: and Bar + data:

+ + """); + } + @Test void renderServerSentEventFragmentStream() { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/SseServerResponse.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/SseServerResponse.java index 1ca6712f64a..52f6b6f6523 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/SseServerResponse.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/SseServerResponse.java @@ -42,6 +42,7 @@ import org.springframework.util.CollectionUtils; import org.springframework.util.MultiValueMap; import org.springframework.web.context.request.async.DeferredResult; import org.springframework.web.servlet.ModelAndView; +import org.springframework.web.util.SseUtils; /** * Implementation of {@link ServerResponse} for sending @@ -149,33 +150,35 @@ final class SseServerResponse extends AbstractServerResponse { @Override public SseBuilder id(String id) { Assert.hasLength(id, "Id must not be empty"); - return field("id", id); + SseUtils.assertNoLineSeparator(id); + this.builder.append("id:").append(id).append('\n'); + return this; } @Override public SseBuilder event(String eventName) { Assert.hasLength(eventName, "Name must not be empty"); - return field("event", eventName); + SseUtils.assertNoLineSeparator(eventName); + this.builder.append("event:").append(eventName).append('\n'); + return this; } @Override public SseBuilder retry(Duration duration) { Assert.notNull(duration, "Duration must not be null"); - String millis = Long.toString(duration.toMillis()); - return field("retry", millis); + this.builder.append("retry:").append(duration.toMillis()).append('\n'); + return this; } @Override public SseBuilder comment(String comment) { - String[] lines = comment.split("\n"); - for (String line : lines) { - field("", line); - } - return this; + return field("", comment); } private SseBuilder field(String name, String value) { - this.builder.append(name).append(':').append(value).append('\n'); + this.builder.append(name).append(':'); + SseUtils.appendFieldValue(name, value, this.builder); + this.builder.append('\n'); return this; } @@ -191,10 +194,7 @@ final class SseServerResponse extends AbstractServerResponse { } private void writeString(String string) throws IOException { - String[] lines = string.split("\n"); - for (String line : lines) { - field("data", line); - } + field("data", string); this.send(); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/UrlFilenameViewController.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/UrlFilenameViewController.java index 07ecbfeee24..9d4d7d3a7a7 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/UrlFilenameViewController.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/UrlFilenameViewController.java @@ -22,8 +22,11 @@ import java.util.concurrent.ConcurrentHashMap; import jakarta.servlet.http.HttpServletRequest; import org.jspecify.annotations.Nullable; +import org.springframework.http.HttpStatus; import org.springframework.util.StringUtils; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.HandlerMapping; +import org.springframework.web.servlet.view.UrlBasedViewResolver; import org.springframework.web.util.ServletRequestPathUtils; /** @@ -150,7 +153,16 @@ public class UrlFilenameViewController extends AbstractUrlViewController { * @see #getSuffix() */ protected String postProcessViewName(String viewName) { - return getPrefix() + viewName + getSuffix(); + return checkViewName(getPrefix() + viewName + getSuffix(), viewName); + } + + private static String checkViewName(String viewNameToUse, String originalViewName) { + if (viewNameToUse.startsWith(UrlBasedViewResolver.REDIRECT_URL_PREFIX) || + viewNameToUse.startsWith(UrlBasedViewResolver.FORWARD_URL_PREFIX)) { + throw new ResponseStatusException( + HttpStatus.BAD_REQUEST, "Rejected viewName '" + originalViewName + "'"); + } + return viewNameToUse; } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java index dd038afa785..159fe369fd0 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java @@ -66,6 +66,7 @@ import org.springframework.web.servlet.View; import org.springframework.web.servlet.ViewResolver; import org.springframework.web.servlet.i18n.AcceptHeaderLocaleResolver; import org.springframework.web.servlet.view.FragmentsRendering; +import org.springframework.web.util.SseUtils; /** * Handler for return values of type: @@ -476,8 +477,9 @@ public class ResponseBodyEmitterReturnValueHandler implements HandlerMethodRetur public byte[] getFragmentContent() { this.writer.flush(); String content = this.outputStream.toString(this.charset); - content = content.replace("\n", "\ndata:"); - return content.getBytes(this.charset); + StringBuilder fragment = new StringBuilder(); + SseUtils.appendFieldValue("data", content, fragment); + return fragment.toString().getBytes(this.charset); } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/SseEmitter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/SseEmitter.java index f0bbe9cd9b4..42ba09c6855 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/SseEmitter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/SseEmitter.java @@ -27,10 +27,10 @@ import org.jspecify.annotations.Nullable; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.server.ServerHttpResponse; -import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import org.springframework.web.servlet.ModelAndView; +import org.springframework.web.util.SseUtils; /** * A specialization of {@link ResponseBodyEmitter} for sending @@ -203,14 +203,14 @@ public class SseEmitter extends ResponseBodyEmitter { @Override public SseEventBuilder id(String id) { - checkEvent(id); + SseUtils.assertNoLineSeparator(id); append("id:").append(id).append('\n'); return this; } @Override public SseEventBuilder name(String name) { - checkEvent(name); + SseUtils.assertNoLineSeparator(name); this.hasName = true; append("event:").append(name).append('\n'); return this; @@ -225,7 +225,7 @@ public class SseEmitter extends ResponseBodyEmitter { @Override public SseEventBuilder comment(String comment) { append(':'); - appendEscaped(comment, "\n:"); + SseUtils.appendFieldValue("", comment, this.sb); append('\n'); return this; } @@ -252,45 +252,16 @@ public class SseEmitter extends ResponseBodyEmitter { return this; } - private static void checkEvent(String content) { - Assert.isTrue(content.indexOf('\n') == -1 && content.indexOf('\r') == -1, - "illegal character '\\n' or '\\r' in event content"); - } - private void writeStringData(String input, @Nullable MediaType mediaType) { if (input.indexOf('\n') == -1 && input.indexOf('\r') == -1) { this.dataToSend.add(new DataWithMediaType(input, mediaType)); } else { - appendEscaped(input, "\ndata:"); + SseUtils.appendFieldValue("data", input, this.sb); saveAppendedText(mediaType); } } - private void appendEscaped(String input, String replacement) { - if (input.indexOf('\n') == -1 && input.indexOf('\r') == -1) { - append(input); - } - else { - int length = input.length(); - for (int i = 0; i < length; i++) { - char c = input.charAt(i); - if (c == '\r') { - if (i + 1 < length && input.charAt(i + 1) == '\n') { - i++; - } - append(replacement); - } - else if (c == '\n') { - append(replacement); - } - else { - append(c); - } - } - } - } - SseEventBuilderImpl append(String text) { this.sb.append(text); return this; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/xslt/XsltView.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/xslt/XsltView.java index c40c3811313..79e064e554f 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/xslt/XsltView.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/xslt/XsltView.java @@ -53,6 +53,7 @@ import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; import org.springframework.util.xml.SimpleTransformErrorListener; import org.springframework.util.xml.TransformerUtils; +import org.springframework.web.servlet.resource.ResourceHandlerUtils; import org.springframework.web.servlet.view.AbstractUrlBasedView; import org.springframework.web.util.WebUtils; @@ -457,12 +458,15 @@ public class XsltView extends AbstractUrlBasedView { protected Source getStylesheetSource() { String url = getUrl(); Assert.state(url != null, "'url' not set"); - if (logger.isDebugEnabled()) { logger.debug("Applying stylesheet [" + url + "]"); } + String location = ResourceHandlerUtils.normalizeInputPath(url); + if (ResourceHandlerUtils.shouldIgnoreInputPath(location)) { + throw new ApplicationContextException("Invalid XSLT stylesheet location '" + url + "'"); + } try { - Resource resource = obtainApplicationContext().getResource(url); + Resource resource = obtainApplicationContext().getResource(location); return new StreamSource(resource.getInputStream(), resource.getURI().toASCIIString()); } catch (IOException ex) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/function/SseServerResponseTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/function/SseServerResponseTests.java index 964238abe81..b95da1ab4e9 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/function/SseServerResponseTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/function/SseServerResponseTests.java @@ -212,6 +212,47 @@ class SseServerResponseTests { assertThat(this.mockResponse.getContentAsString()).isEqualTo(expected); } + @Test + void sendStringWithCarriageReturn() throws Exception { + String body = "line1\rline2\r\nline3"; + ServerResponse response = ServerResponse.sse(sse -> { + try { + sse.send(body); + } + catch (IOException ex) { + throw new UncheckedIOException(ex); + } + }); + + ServerResponse.Context context = Collections::emptyList; + + ModelAndView mav = response.writeTo(this.mockRequest, this.mockResponse, context); + assertThat(mav).isNull(); + + String expected = "data:line1\ndata:line2\ndata:line3\n\n"; + assertThat(this.mockResponse.getContentAsString()).isEqualTo(expected); + } + + @Test + void commentWithCarriageReturn() throws Exception { + ServerResponse response = ServerResponse.sse(sse -> { + try { + sse.comment("line1\rline2").send(); + } + catch (IOException ex) { + throw new UncheckedIOException(ex); + } + }); + + ServerResponse.Context context = Collections::emptyList; + + ModelAndView mav = response.writeTo(this.mockRequest, this.mockResponse, context); + assertThat(mav).isNull(); + + String expected = ":line1\n:line2\n\n"; + assertThat(this.mockResponse.getContentAsString()).isEqualTo(expected); + } + @Test // gh-34608 void sendHeartbeat() throws Exception { ServerResponse response = ServerResponse.sse(sse -> { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/UrlFilenameViewControllerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/UrlFilenameViewControllerTests.java index d32142db1bb..9344528d75a 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/UrlFilenameViewControllerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/UrlFilenameViewControllerTests.java @@ -22,8 +22,10 @@ import java.util.stream.Stream; import org.junit.jupiter.api.Named; import org.junit.jupiter.api.Test; +import org.springframework.http.HttpStatus; import org.springframework.ui.ModelMap; import org.springframework.util.AntPathMatcher; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.ModelAndView; @@ -34,6 +36,7 @@ import org.springframework.web.testfixture.servlet.MockHttpServletResponse; import org.springframework.web.util.ServletRequestPathUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * @author Juergen Hoeller @@ -75,6 +78,24 @@ class UrlFilenameViewControllerTests { assertThat(mv.getModel()).isEmpty(); } + @PathPatternsParameterizedTest + void withRedirectPrefix(Function requestFactory) { + UrlFilenameViewController controller = new UrlFilenameViewController(); + MockHttpServletRequest request = requestFactory.apply("/redirect:index"); + assertThatExceptionOfType(ResponseStatusException.class) + .isThrownBy(() -> controller.handleRequest(request, new MockHttpServletResponse())) + .satisfies(ex -> assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST)); + } + + @PathPatternsParameterizedTest + void withForwardPrefix(Function requestFactory) { + UrlFilenameViewController controller = new UrlFilenameViewController(); + MockHttpServletRequest request = requestFactory.apply("/forward:index"); + assertThatExceptionOfType(ResponseStatusException.class) + .isThrownBy(() -> controller.handleRequest(request, new MockHttpServletResponse())) + .satisfies(ex -> assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST)); + } + @PathPatternsParameterizedTest void withPrefixAndSuffix(Function requestFactory) throws Exception { UrlFilenameViewController controller = new UrlFilenameViewController(); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/FragmentRenderingStreamTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/FragmentRenderingStreamTests.java index a5fec8bd397..f478dd9dfa9 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/FragmentRenderingStreamTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/FragmentRenderingStreamTests.java @@ -146,6 +146,31 @@ class FragmentRenderingStreamTests { """)); } + @Test + void escapeViewFragment() throws Exception { + MethodParameter type = on(TestController.class).resolveReturnType(SseEmitter.class); + + SseEmitter emitter = new SseEmitter(); + this.handler.handleReturnValue(emitter, type, new ModelAndViewContainer(), webRequest); + + assertThat(this.request.isAsyncStarted()).isTrue(); + assertThat(this.response.getStatus()).isEqualTo(200); + + ModelAndView mav1 = new ModelAndView("fragment1", Map.of("foo", "Foo\n and Bar")); + + emitter.send(SseEmitter.event().data(mav1)); + + assertThat(this.response.getContentType()).isEqualTo("text/event-stream"); + assertThat(this.response.getContentAsString()).isEqualTo((""" + event:fragment1 + data:

+ data: Hello Foo + data: and Bar + data:

+ + """)); + } + @SuppressWarnings({"unused", "DataFlowIssue"}) private static class TestController {