Skip to content

Return 400 Bad Request when request body converter returns incompatible type - #2562

Open
BabalolaBrainiac wants to merge 1 commit into
spring-projects:mainfrom
BabalolaBrainiac:issue-1194-uri-list-400
Open

BabalolaBrainiac wants to merge 1 commit into
spring-projects:mainfrom
BabalolaBrainiac:issue-1194-uri-list-400

Conversation

@BabalolaBrainiac

Copy link
Copy Markdown

Fixes #1194.
|
When text/uri-list is posted to a non-association endpoint, UriListHttpMessageConverter returns a CollectionModel of links. The argument resolver then fails with an internal exception mapped to 500.
This change validates the converter output type and throws HttpMessageNotReadableException (400) instead.

  • Added type check in PersistentEntityResourceHandlerMethodArgumentResolver
  • Added unit test rejectsRequestBodyIfConverterReturnsIncompatibleType

  • You have read the Spring Data contribution guidelines.
  • You use the code formatters provided here and have them applied to your changes. Don't submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.
  • You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

@BabalolaBrainiac
BabalolaBrainiac force-pushed the issue-1194-uri-list-400 branch from 8cd6da9 to a6390d9 Compare May 17, 2026 20:18
…le type.

When text/uri-list is posted to a non-association endpoint,
UriListHttpMessageConverter returns a CollectionModel instead of the
target domain type. The argument resolver then fails with an internal
exception mapped to 500.

This change validates the converter output type and throws
HttpMessageNotReadableException (400) instead.

Fixes spring-projects#1194

Signed-off-by: Babalola Opeyemi Daniel <babalolaopedaniel@gmail.com>
if (!domainType.isInstance(newObject)) {
throw new HttpMessageNotReadableException(String.format(ERROR_MESSAGE, domainType), request);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the previous check throw the same exception with the same message. They could be combined into a single guard:

if (newObject == null || !domainType.isInstance(newObject)) {
    throw new HttpMessageNotReadableException(String.format(ERROR_MESSAGE, domainType), request);
}

Also, the error message doesn't distinguish the type mismatch from a null result:

ERROR_MESSAGE = "Could not read an object of type %s from the request" is used for both the null case and the type mismatch case. For the type mismatch, a more informative message would help API consumers diagnose the problem, e.g.:

"Expected an object of type %s but the request body was interpreted as %s. Check the Content-Type header."

This is especially useful when the client sends text/uri-list to a non-association endpoint — the current message gives no hint about what went wrong.


doReturn(CollectionModel.empty()).when(converter).read(Mockito.any(Class.class), Mockito.any(HttpInputMessage.class));

assertThatExceptionOfType(HttpMessageNotReadableException.class).isThrownBy(() -> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add .withMessageContaining(...) to the test assertion for precision.

The test only asserts that HttpMessageNotReadableException is thrown, but doesn't verify the message content. While this is acceptable for a minimal test, asserting .withMessageContaining(...) would make the test more precise and guard against accidentally catching a different HttpMessageNotReadableException thrown earlier in the call chain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: waiting-for-triage An issue we've not yet triaged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Posting text/uri-list to the wrong location results in a 500 [DATAREST-818]

4 participants