- Notifications
You must be signed in to change notification settings - Fork 38.8k
Description
Overview
In #31769 (comment), @jandroalvarez brought it to our attention that 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.resolveMediaTypes.
HeaderContentNegotiationStrategy.resolveMediaTypes() still allows the InvalidMimeTypeException to propagate as-is without wrapping it in an HttpMediaTypeNotAcceptableException.
We should therefore catch both InvalidMediaTypeException and InvalidMimeTypeException in HeaderContentNegotiationStrategy.resolveMediaTypes() and wrap the exception in an HttpMediaTypeNotAcceptableException.
Proposed Fix and Test
catch (InvalidMediaTypeException | InvalidMimeTypeException ex) in HeaderContentNegotiationStrategy.resolveMediaTypes().
@Test void resolveMediaTypesWithTooManyElements() throws Exception { String acceptHeaderValue = "text/plain,".repeat(50); this.servletRequest.addHeader("Accept", acceptHeaderValue); assertThat(this.strategy.resolveMediaTypes(this.webRequest)).hasSize(50); 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"); }