Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Http header Content-Type updated can be ignored in incoming request matching #45058

Closed
alexandre-baron opened this issue Dec 11, 2024 · 7 comments · Fixed by #45085
Closed

Http header Content-Type updated can be ignored in incoming request matching #45058

alexandre-baron opened this issue Dec 11, 2024 · 7 comments · Fixed by #45085
Labels
area/rest kind/bug Something isn't working
Milestone

Comments

@alexandre-baron
Copy link

Describe the bug

Hi. In a context where a ContainerRequestFilter update some properties of the HTTP request:

  • if a query param version is present, update the Accept and the Content-Type HTTP headers
@Provider
@PreMatching
class VersionFilter : ContainerRequestFilter {
    override fun filter(requestContext: ContainerRequestContext) {
        val versionParam = requestContext.uriInfo.queryParameters.get("version")
        if (versionParam != null) {
            if (versionParam.first().equals("1")) {
                if (requestContext.getHeaderString("Accept") != null) {
                    // Accept header update
                    requestContext.headers.replace("Accept", listOf("application/vnd.acme.v1+json"))
                }
                if (requestContext.getHeaderString("Content-Type") != null) {
                    // Content-Type header update
                    requestContext.headers.replace("Content-Type", listOf("application/vnd.acme.v1+json"))
                }
            }
        }
    }
}

With a controller reacting on /api/versions with several methods:

  • GET /api/versions/{id} produces application/json
  • GET /api/versions/{id} produces application/vnd.acme.v1+json
  • POST /api/versions consumes application/json
  • POST /api/versions consumes application/vnd.acme.v1+json

And payload schema for:

  • application/json is an object with a field id
  • application/application/vnd.acme.v1+json is an object with a field name

When the GET /api/versions/{id} is called with:

###
GET http://localhost:8080/api/versions/1
Accept: application/json

# Returns
HTTP/1.1 200 OK
content-length: 10
Content-Type: application/json;charset=UTF-8

{
  "id": "1"
}

###
GET http://localhost:8080/api/versions/1
Accept: application/vnd.acme.v1+json

# Returns
HTTP/1.1 200 OK
content-length: 12
Content-Type: application/vnd.acme.v1+json;charset=UTF-8

{
  "name": "1"
}

###
GET http://localhost:8080/api/versions/1?version=1
Accept: application/json

# Returns
HTTP/1.1 200 OK
content-length: 12
Content-Type: application/vnd.acme.v1+json;charset=UTF-8

{
  "name": "1"
}

The usage of version parameter work and change the response content-type.

When the POST /api/versions is called with:

###
POST http://localhost:8080/api/versions
Content-Type: application/json

{
  "id": "1"
}

# Returns
HTTP/1.1 201 Created
Location: http://localhost:8080/api/version/1
content-length: 0

###
POST http://localhost:8080/api/versions
Content-Type: application/vnd.acme.v1+json

{
  "name": "1"
}

# Returns
HTTP/1.1 201 Created
Location: http://localhost:8080/api/version/1
content-length: 0

###
POST http://localhost:8080/api/versions?version=1
Content-Type: application/json

{
  "name": "1"
}

# Returns
HTTP/1.1 400 Bad Request
Content-Type: application/json;charset=UTF-8
content-length: 63

{
  "message": "Incorrect format for object Class",
  "property": "id"
}

The usage of version parameter throw an HTTP error, that indicate the web server try to map the request payload to the schema of application/json.

Behavior is not consistent between update of Content-Type header and Accept header.

After investigation, I fall in the org.jboss.resteasy.reactive.server.handlers.MediaTypeMapper that is called when two methods have the same path.
Its class javadoc indicates

Handler that deals with the case when two methods have the same path, and it needs to select based on content type.
This is not super optimised, as it is not a common case. Most apps won't every use this handler.

Omitting the non-reassuring nature of the last line, I find this line (cf.

String contentType = requestContext.serverRequest().getRequestHeader(HttpHeaders.CONTENT_TYPE);
).

It is called to get the content type of the request payload.

But, without javadoc on the serverRequest() and getRequestHeader() methods to indicate theirs goal, I deduce that it retrieve orginal HTTP headers and not updated ones.

Replace this code by requestContext.getRequestHeaders().getHeaderString(HttpHeaders.CONTENT_TYPE) seems to search in the updated request headers instead of the originals ones.

But I can't be sure of this fix, as the javadoc on these interfaces is missing.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@alexandre-baron alexandre-baron added the kind/bug Something isn't working label Dec 11, 2024
Copy link

quarkus-bot bot commented Dec 11, 2024

/cc @geoand (kotlin)

@geoand
Copy link
Contributor

geoand commented Dec 11, 2024

What version of Quarkus are you seeing this issue for?

Better yet, if you could attach a small sample that we can use to see the problem in action, it would be great

@geoand geoand added the triage/needs-feedback We are waiting for feedback. label Dec 11, 2024
@alexandre-baron
Copy link
Author

I use the 3.15.1.
But looking at the main branch code, it seems to be present yet.

The filter code is already present in the bug description.

And my controller looks like

@ApplicationScoped
@Path("/api/versions")
class VersioningResource {

    @GET
    @Path("/{id}")
    fun get1(
        @PathParam("id") id: String
    ): Version1 {
        return Version1(id)
    }

    @GET
    @Path("/{id}")
    @Produces("application/vnd.acme.v1+json")
    fun get2(
        @PathParam("id") id: String
    ): Version2 {
        return Version2(id)
    }

    @POST
    @Path("/search")
    fun search1(
        search: Version1,
        @QueryParam("pageSize") pageSize: Int
    ): List<Version1> {
        return listOf(
            Version1("1"),
            Version1("2"),
            Version1("3")
        )
    }

    @POST
    @Path("/search")
    @Consumes("application/vnd.acme.v1+json")
    @Produces("application/vnd.acme.v1+json")
    fun search2(
        search: Version2,
        @QueryParam("pageSize") pageSize: Int
    ): List<Version2> {
        return listOf(
            Version2("1"),
            Version2("2"),
            Version2("3")
        )
    }

    @POST
    fun create1(
        body: Version1
    ): Response {
        return Response.created(URI.create("/api/version/${body.id}")).build()
    }

    @POST
    @Consumes("application/vnd.acme.v1+json")
    fun create2(
        body: Version2
    ): Response {
        return Response.created(URI.create("/api/version/${body.name}")).build()
    }

}

data class Version1(
    val id: String
)

data class Version2(
    val name: String
)

@geoand
Copy link
Contributor

geoand commented Dec 11, 2024

If you could attach the sample project that has that code you are showing, it would be marvelous

@alexandre-baron
Copy link
Author

@geoand geoand removed the triage/needs-feedback We are waiting for feedback. label Dec 11, 2024
@geoand
Copy link
Contributor

geoand commented Dec 11, 2024

🙏🏽

@geoand
Copy link
Contributor

geoand commented Dec 12, 2024

Thanks a lot!

#45085 should fix it

gastaldi added a commit that referenced this issue Dec 12, 2024
Use Content-Type header from PreMatching filter during media type negotiation
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants