From 09f210f5f749e475385163be322ae1fef17c699e Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Tue, 19 Mar 2024 14:15:19 +0100 Subject: [PATCH 1/3] Polish wording in reference manual --- framework-docs/modules/ROOT/pages/core/aot.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework-docs/modules/ROOT/pages/core/aot.adoc b/framework-docs/modules/ROOT/pages/core/aot.adoc index a07ea864dba8..33c3b20af066 100644 --- a/framework-docs/modules/ROOT/pages/core/aot.adoc +++ b/framework-docs/modules/ROOT/pages/core/aot.adoc @@ -167,7 +167,7 @@ Kotlin:: ---- ====== -WARNING: Kotlin class names with backticks using invalid Java identifiers (not starting by a letter, containing spaces, etc.) are not supported. +WARNING: Kotlin class names with backticks that use invalid Java identifiers (not starting with a letter, containing spaces, etc.) are not supported. Since there isn't any particular condition on this class, `dataSourceConfiguration` and `dataSource` are identified as candidates. The AOT engine will convert the configuration class above to code similar to the following: From 836a0b3a40f281a642c122dcfb9976603d692e54 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Tue, 19 Mar 2024 14:19:01 +0100 Subject: [PATCH 2/3] Polishing --- ...HeaderContentNegotiationStrategyTests.java | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/spring-web/src/test/java/org/springframework/web/accept/HeaderContentNegotiationStrategyTests.java b/spring-web/src/test/java/org/springframework/web/accept/HeaderContentNegotiationStrategyTests.java index b5115cf9662e..86caa7cedc47 100644 --- a/spring-web/src/test/java/org/springframework/web/accept/HeaderContentNegotiationStrategyTests.java +++ b/spring-web/src/test/java/org/springframework/web/accept/HeaderContentNegotiationStrategyTests.java @@ -30,7 +30,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** - * Test fixture for HeaderContentNegotiationStrategy tests. + * Tests for {@link HeaderContentNegotiationStrategy}. * * @author Rossen Stoyanchev * @author Juergen Hoeller @@ -49,31 +49,27 @@ void resolveMediaTypes() throws Exception { this.servletRequest.addHeader("Accept", "text/plain; q=0.5, text/html, text/x-dvi; q=0.8, text/x-c"); List mediaTypes = this.strategy.resolveMediaTypes(this.webRequest); - assertThat(mediaTypes).hasSize(4); - assertThat(mediaTypes.get(0).toString()).isEqualTo("text/html"); - assertThat(mediaTypes.get(1).toString()).isEqualTo("text/x-c"); - assertThat(mediaTypes.get(2).toString()).isEqualTo("text/x-dvi;q=0.8"); - assertThat(mediaTypes.get(3).toString()).isEqualTo("text/plain;q=0.5"); + assertThat(mediaTypes).map(Object::toString) + .containsExactly("text/html", "text/x-c", "text/x-dvi;q=0.8", "text/plain;q=0.5"); } - @Test // SPR-14506 - public void resolveMediaTypesFromMultipleHeaderValues() throws Exception { + @Test // gh-19075 + void resolveMediaTypesFromMultipleHeaderValues() throws Exception { this.servletRequest.addHeader("Accept", "text/plain; q=0.5, text/html"); this.servletRequest.addHeader("Accept", "text/x-dvi; q=0.8, text/x-c"); List mediaTypes = this.strategy.resolveMediaTypes(this.webRequest); - assertThat(mediaTypes).hasSize(4); - assertThat(mediaTypes.get(0).toString()).isEqualTo("text/html"); - assertThat(mediaTypes.get(1).toString()).isEqualTo("text/x-c"); - assertThat(mediaTypes.get(2).toString()).isEqualTo("text/x-dvi;q=0.8"); - assertThat(mediaTypes.get(3).toString()).isEqualTo("text/plain;q=0.5"); + assertThat(mediaTypes).map(Object::toString) + .containsExactly("text/html", "text/x-c", "text/x-dvi;q=0.8", "text/plain;q=0.5"); } @Test void resolveMediaTypesParseError() { this.servletRequest.addHeader("Accept", "textplain; q=0.5"); - assertThatExceptionOfType(HttpMediaTypeNotAcceptableException.class).isThrownBy(() -> - this.strategy.resolveMediaTypes(this.webRequest)); + assertThatExceptionOfType(HttpMediaTypeNotAcceptableException.class) + .isThrownBy(() -> this.strategy.resolveMediaTypes(this.webRequest)) + .withMessageStartingWith("Could not parse 'Accept' header") + .withMessageContaining("Invalid mime type"); } } From ef02f0bad87ddd4672c88f9018333e0fb9ba5cde Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Tue, 19 Mar 2024 14:20:40 +0100 Subject: [PATCH 3/3] Wrap InvalidMimeTypeException in HttpMediaTypeNotAcceptableException The fix for #31254 resulted in an InvalidMimeTypeException being thrown by MimeTypeUtils.sortBySpecificity() instead of an IllegalArgumentException. However, InvalidMimeTypeException extends IllegalArgumentException. Consequently, the change from IllegalArgumentException to InvalidMimeTypeException did not result in the desired effect in HeaderContentNegotiationStrategy. HeaderContentNegotiationStrategy.resolveMediaTypes() still allows the InvalidMimeTypeException to propagate as-is without wrapping it in an HttpMediaTypeNotAcceptableException. To address this issue, this commit catches InvalidMediaTypeException and InvalidMimeTypeException in HeaderContentNegotiationStrategy and wraps the exception in an HttpMediaTypeNotAcceptableException. See gh-31254 See gh-31769 Closes gh-32483 --- .../HeaderContentNegotiationStrategy.java | 5 +++-- ...HeaderContentNegotiationStrategyTests.java | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/accept/HeaderContentNegotiationStrategy.java b/spring-web/src/main/java/org/springframework/web/accept/HeaderContentNegotiationStrategy.java index 9ef86aabfd1a..32bf811d30ca 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/HeaderContentNegotiationStrategy.java +++ b/spring-web/src/main/java/org/springframework/web/accept/HeaderContentNegotiationStrategy.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2024 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. @@ -23,6 +23,7 @@ import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; import org.springframework.util.CollectionUtils; +import org.springframework.util.InvalidMimeTypeException; import org.springframework.util.MimeTypeUtils; import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.context.request.NativeWebRequest; @@ -55,7 +56,7 @@ public List resolveMediaTypes(NativeWebRequest request) MimeTypeUtils.sortBySpecificity(mediaTypes); return !CollectionUtils.isEmpty(mediaTypes) ? mediaTypes : MEDIA_TYPE_ALL_LIST; } - catch (InvalidMediaTypeException ex) { + catch (InvalidMediaTypeException | InvalidMimeTypeException ex) { throw new HttpMediaTypeNotAcceptableException( "Could not parse 'Accept' header " + headerValues + ": " + ex.getMessage()); } diff --git a/spring-web/src/test/java/org/springframework/web/accept/HeaderContentNegotiationStrategyTests.java b/spring-web/src/test/java/org/springframework/web/accept/HeaderContentNegotiationStrategyTests.java index 86caa7cedc47..f952fb0b9efa 100644 --- a/spring-web/src/test/java/org/springframework/web/accept/HeaderContentNegotiationStrategyTests.java +++ b/spring-web/src/test/java/org/springframework/web/accept/HeaderContentNegotiationStrategyTests.java @@ -34,6 +34,7 @@ * * @author Rossen Stoyanchev * @author Juergen Hoeller + * @author Sam Brannen */ class HeaderContentNegotiationStrategyTests { @@ -63,6 +64,27 @@ void resolveMediaTypesFromMultipleHeaderValues() throws Exception { .containsExactly("text/html", "text/x-c", "text/x-dvi;q=0.8", "text/plain;q=0.5"); } + @Test // gh-32483 + void resolveMediaTypesWithMaxElements() throws Exception { + String acceptHeaderValue = "text/plain, text/html,".repeat(25); + this.servletRequest.addHeader("Accept", acceptHeaderValue); + List mediaTypes = this.strategy.resolveMediaTypes(this.webRequest); + + assertThat(mediaTypes).hasSize(50); + assertThat(mediaTypes.stream().map(Object::toString).distinct()) + .containsExactly("text/plain", "text/html"); + } + + @Test // gh-32483 + void resolveMediaTypesWithTooManyElements() { + String acceptHeaderValue = "text/plain,".repeat(51); + this.servletRequest.addHeader("Accept", acceptHeaderValue); + assertThatExceptionOfType(HttpMediaTypeNotAcceptableException.class) + .isThrownBy(() -> this.strategy.resolveMediaTypes(this.webRequest)) + .withMessageStartingWith("Could not parse 'Accept' header") + .withMessageEndingWith("Too many elements"); + } + @Test void resolveMediaTypesParseError() { this.servletRequest.addHeader("Accept", "textplain; q=0.5");