Skip to content

Fix templated Content-Type handling in SpringEncoder - #1406

Open
yashdotdev13 wants to merge 1 commit into
spring-cloud:mainfrom
yashdotdev13:issue-116-dynamic-content-type
Open

yashdotdev13 wants to merge 1 commit into
spring-cloud:mainfrom
yashdotdev13:issue-116-dynamic-content-type

Conversation

@yashdotdev13

Copy link
Copy Markdown
Contributor

Description

Fixes #116

SpringEncoder previously attempted to parse an unresolved templated Content-Type value, such as {Content-Type}, using MediaType.valueOf(...), which resulted in an invalid media type exception.

This change handles unresolved Content-Type templates by skipping media type parsing and removing the unresolved template before creating the HttpHeaders passed to Spring's message converters.

Changes

  • Skip MediaType parsing for unresolved Content-Type templates.
  • Remove unresolved Content-Type templates from headers passed to message converters.
  • Add a regression test for encoding requests with a templated Content-Type header.

Testing

  • Added a regression test in SpringEncoderTests.
  • Verified the targeted unit and integration tests.
  • Ran the full build successfully:
mvn clean verify

Signed-off-by: 98001yash <yashchauhan.gaya@gmail.com>

@kdelay kdelay left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

On 86bda63, JDK 17: core module 426 tests / 0 failures. Two gaps before this closes #116:

  1. The template is dropped, not deferred: the headers get overwritten with the converter's, so {Content-Type} plus a POJO body ends up application/json, and resolve(Content-Type=application/xml) still gives application/json. Re-applying the saved template after request.headers(newHeaders) makes resolve give application/xml, suite still 426/0.

  2. application/{ct} still throws InvalidMediaTypeException at SpringEncoder:99.

The new test asserts only body() != null, so it passes under (1).

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dynamically set Content-Type via @RequestHeader

3 participants