This PR fixes condition for "Too many elements" in MimeTypeUtils.sortBySpecificity() that seems to have been changed accidentally in 05c3ffb2fbdf358c6a23309a3118b0a64ecb4b40 to align with its Javadoc again.

See gh-31254

Comment From: sbrannen

Good catch!

Although this is very minor, it is technically a regression.

So I've labeled it as such, and we'll back port it to 6.0.x

Comment From: jandroalvarez

@sbrannen in #31254 it was notified MimeTypeUtils.sortBySpecificity was throwing an IllegalArgumentException when the number of MimeTypes in the Accept header was greater than 50.

To fix it, the proposed change was to modify HeaderContentNegotiationStrategy to catch IllegalArgumentExceptiion instead of InvalidMediaTypeException (InvalidMediaTypeException is a sub-type of IllegalArgumentException).

Instead, the assert that was throwing the IllegalArgumentException was replaced by an if condition that throws InvalidMimeTypeException which is a sub-type of IllegalArgumentException.

Therefore, the same runtime problem is being reproduced. Only changed IllegalArgumentException with InvalidMimeTypeException.

This problem was reported in this issue and it was just changed the if statement replacing the >= with >.

Why haven't you changed the catch clause as originally requested in #31254? The same runtime crash is still happening.

Comment From: sbrannen

@jandroalvarez, please note that this PR was closed over 3 months ago in 6.1.2.

If you believe you have discovered a further regression or bug, please open a new issue.

Thanks

Comment From: sbrannen

To answer your question, it appears there may have been an oversight.

Why haven't you changed the catch clause as originally requested in #31254? The same runtime crash is still happening.

I suppose you're proposing to change what we currently have from:

catch (InvalidMediaTypeException ex) {
    throw new HttpMediaTypeNotAcceptableException(
            "Could not parse 'Accept' header " + headerValues + ": " + ex.getMessage());
}

To:

catch (InvalidMediaTypeException | InvalidMimeTypeException ex) {
    throw new HttpMediaTypeNotAcceptableException(
            "Could not parse 'Accept' header " + headerValues + ": " + ex.getMessage());
}

Or:

catch (IllegalArgumentException ex) {
    throw new HttpMediaTypeNotAcceptableException(
            "Could not parse 'Accept' header " + headerValues + ": " + ex.getMessage());
}

I'm not convinced that we should catch IllegalArgumentException there.

But if you're effectively suggesting the former (InvalidMediaTypeException | InvalidMimeTypeException), then please create a new issue.

Cheers!

Comment From: sbrannen

@jandroalvarez, to ensure the issue you've reported does not get lost, I am going to go ahead and create a new issue based on your input now.

So, there's no need for you to create a new issue.

  • See #32483