From da799bc5192686f5732cec6c52c80c37995233dc 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] 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 (cherry picked from commit ef02f0bad87ddd4672c88f9018333e0fb9ba5cde) --- .../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 9ef86aabfd1..32bf811d30c 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.HttpHeaders; 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 class HeaderContentNegotiationStrategy implements ContentNegotiationStrat 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 08a437f1037..777f2904166 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 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; * * @author Rossen Stoyanchev * @author Juergen Hoeller + * @author Sam Brannen */ public class HeaderContentNegotiationStrategyTests { @@ -63,6 +64,27 @@ public class HeaderContentNegotiationStrategyTests { .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 public void resolveMediaTypesParseError() throws Exception { this.servletRequest.addHeader("Accept", "textplain; q=0.5");