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

[logbook-ktor-client] removes Content-Type request header #1821

Closed
grassehh opened this issue Apr 29, 2024 · 2 comments
Closed

[logbook-ktor-client] removes Content-Type request header #1821

grassehh opened this issue Apr 29, 2024 · 2 comments
Labels

Comments

@grassehh
Copy link

grassehh commented Apr 29, 2024

Description

When sending requests using Ktor client with the LogbookClient plugin from logbook-ktor-client installed, the Content-Type header is removed from the request.
Another side effect of this issue is that JacksonJsonFieldBodyFilter does not work properly as it checks the content type.

Expected Behavior

The Content-Type header should be preserved when sending the request.

Actual Behavior

The Content-Typeheader is removed from the request.

Possible Fix

There are two places to fix.

  • The first is in the LogbookClient class itself. This line should return
it
  • The second place is in the ClientRequest class.
    The getHeaders() and getContentType() method should try to respectively append and retrieve the Content-Type header from the OutgoingContent.contentType property when applicable (this one is necessary in order for features like JacksonJsonFieldBodyFilter to work properly).
    Example:
override fun getHeaders(): HttpHeaders = HttpHeaders.of(request.headers.build().toMap())
        .also {
            if (it.contains(ContentType).not() && request.body is OutgoingContent) return it.update(
                ContentType,
                (request.body as OutgoingContent).contentType.toString()
            )
        }

override fun getContentType(): String? =
        request.contentType()?.toString()?.substringBefore(";") ?: (request.body as? OutgoingContent)?.contentType?.toString()

Explainations: For some reasons, Ktor removes the Content-Type header from the request object and pushes it into the OutgoingContent.
Thus the Content-Type is lost when Logbook replaces the content with a fresh new one by using ByteArrayContent(content), and the ClientRequest needs to retrieve the information from the OutgoingContent instead of the headers object.

Steps to Reproduce

The test will fail. If you uninstall LogbookClient, the test will pass.

Context

I was writing integration tests that check that my request contained the Content-Type: application/json header and it failed.

Your Environment

Ktor 2.3.10
Logbook 3.8.0

@grassehh grassehh added the Bug label Apr 29, 2024
@grassehh grassehh changed the title logbook-ktor-client removes Content-Type request header [logbook-ktor-client] removes Content-Type request header May 2, 2024
@kasmarian
Copy link
Member

@grassehh please see #1833 with the fix that you suggested. And thank you very much for the detailed explanation of the issue and your investigation.

@kasmarian
Copy link
Member

The fix will be in the next release, thank you for the detailed issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants