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

fix: multipart response content-type header remove new lines and tabs #69

Conversation

TomerFi
Copy link
Member

@TomerFi TomerFi commented Jul 2, 2023

Signed-off-by: Tomer Figenblat tfigenbl@redhat.com

The Content-Type header when using multipart type responses contains a new line and a tab for including the attributes map. This breaks Java's built-in http client.

More info can be found in https://github.com/TomerFi/backend-multipart.
This PR resolves https://issues.redhat.com/browse/APPENG-1864.

Fixes #71

@TomerFi TomerFi force-pushed the fix-multipart-header-special-chars branch from a4030f6 to d672f70 Compare July 2, 2023 12:00
@ilan-pinto ilan-pinto requested a review from ruromero July 2, 2023 12:13
@ruromero
Copy link
Collaborator

ruromero commented Jul 2, 2023

@TomerFi The Java Client by default uses HTTP/2 but if you use HTTP/1.1 it just works. This is the updated version of your reproducer:

    var request = HttpRequest.newBuilder(URI.create(String.format("%s/api/v3/dependency-analysis/maven", backend)))
      .setHeader("Accept", "multipart/mixed")
      .version(Version.HTTP_1_1)
      .setHeader("Content-Type", "text/vnd.graphviz")
      .POST(HttpRequest.BodyPublishers.ofInputStream(() -> App.class.getClassLoader().getResourceAsStream("sut_dot_graph")))
      .build();

In the Go client you are also using HTTP/1.1 so I think HTTP/2 not a requirement for this API.

Anyway, it seems the HTTP/2 protocol is more restrictive with the boundary definition so I will investigate if this can be fixed within the Camel source itself.

@TomerFi
Copy link
Member Author

TomerFi commented Jul 3, 2023

@ilan-pinto I don't think setting our clients to use HTTP/1.1 instead HTTP/2 is the right call.
Can you please help getting this merged? It blocks our API Libs development.

@ruromero
Copy link
Collaborator

ruromero commented Jul 3, 2023

I've created an issue in Camel jira https://issues.apache.org/jira/browse/CAMEL-19568

Signed-off-by: Tomer Figenblat <tfigenbl@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multipart/Mixed accept type breaks Http 2.0 clients
2 participants