Kong/unirest-java

Configured HttpClient.Version ignored

JamesBoon opened this issue ยท 10 comments

Describe the bug
Setting the http version does nothing, as it is not read from the configuration in JavaClient.

To Reproduce

Unirest.config()
        .version(HttpClient.Version.HTTP_1_1);

HttpResponse<String> response = Unirest.post("https://httpbin.org/post")
        .body("test body")
        .asString();

// Should NOT contain "Connection", "Upgrade" and "Http2-Settings" header
System.out.println(response.getBody());

Expected behavior
Http version 1.1 should be used if configured and no HTTP2 header sent.

Environmental Data:

  • Unirest version 4.2.1

Additional context
I think changing the line .version(HttpClient.Version.HTTP_2) to something like .version(config.getVersion()) should fix the problem.

In addition, it would be very practical if the version could be set individually for each request.
Just like this:

HttpResponse<String> response = Unirest.post("https://httpbin.org/post")
        .version(HttpClient.Version.HTTP_1_1)
        .body("test body")
        .asString();

Should I create a new feature request for this?

ryber commented

just released 4.2.5 with this fix. I just did the root config for now, per request is a but more work.

Thank you! The commits look good.
What do you think, how long will it take until 4.2.5 will be available in Maven Central? EDIT: It is available - just not visible on the webpage ๐Ÿ™„

And should I create a new feature request for the "per request" configuration? Or do you find that feature (request) unnecessary?

ryber commented

Yea Maven central is a maze, the artifacts will typically show up for download in under 1 hour (I assume there is some cache that needs to expire), the website updates about once per day.

As for the per-request. We can just leave this issue open. But what I would like to understand is the reason folk would want to do that? It was my understanding (probably wrong) that HTTP2 was backwards compatible and degraded gracefully to 1.1. You would think the extra headers would just be ignored. What happened that caused you to even look at this?

I have to make calls to an old legacy server that denies access as soon as I send the http2 headers. (Don't ask me why!)
It was actually a bit of a pain to find the cause for this behavior ๐Ÿ˜…

Seriously, @ryber, this is amazing. I just came here and thought I would ask you if you need any help with the implementation, like creating a PR.
And now you're already done!

ryber commented

I'm trying to figure out if it actually deployed, maven central threw a weird error at the very end after it uploaded the artifacts. Its kind of a mess

ryber commented

Oh good, I think we can close this then if it works for you

It works perfectly!