Re: RFR: 8283544: HttpClient GET method adds Content-Length: 0 header [v2]
On Wed, 30 Mar 2022 08:29:40 GMT, Conor Cleary wrote: >> **Issue** >> When using the `HttpClient.send()` to send a GET request created using the >> `HttpRequest.newBuilder()`, a `Content-length: 0` header is set. This >> behaviour causes issues with many services as a body related header is >> usually not expected to be included with a GET request. >> >> **Solution** >> `Http1Request.java` was modified so that when the request method is a GET, a >> `Content-length` header is not added to the request. However, if a developer >> chooses to include a body in a GET request (though it is generally >> considered bad practice), a `Content-length` header with the appropriate >> value will be added. > > Conor Cleary has updated the pull request incrementally with one additional > commit since the last revision: > > 8283544: Removed testing comments On another note, I do a small check to see if the value given in the Content-length header is correct and matches up with the actual length of the body. That check is probably outside of the scope of this test and probably unecessary so I was considering removing it unless there are any arguments for keeping it. - PR: https://git.openjdk.java.net/jdk/pull/8017
Re: RFR: 8283544: HttpClient GET method adds Content-Length: 0 header [v2]
On Tue, 29 Mar 2022 17:09:37 GMT, Daniel Fuchs wrote: > I would not expect that throwing an AssertionError in the handler on the > server side would make the client (and the test) fail (except maybe in > timeout?). I'd suggest printing a message on System.err and sending a > different error code (like 400 for instance) if expectations are not met, and > have the client side checks that it receives 200. @dfuch Yes you're correct. I've just verified there that when there is a failure in the test's current state it is the handleResponse() method that throws an IO Exception rather than an AssertionError from the test methods. I think as you suggest, handling the Test Errors by response codes sent from the server makes the most sense in this case. - PR: https://git.openjdk.java.net/jdk/pull/8017
Re: RFR: 8283544: HttpClient GET method adds Content-Length: 0 header [v2]
> **Issue** > When using the `HttpClient.send()` to send a GET request created using the > `HttpRequest.newBuilder()`, a `Content-length: 0` header is set. This > behaviour causes issues with many services as a body related header is > usually not expected to be included with a GET request. > > **Solution** > `Http1Request.java` was modified so that when the request method is a GET, a > `Content-length` header is not added to the request. However, if a developer > chooses to include a body in a GET request (though it is generally considered > bad practice), a `Content-length` header with the appropriate value will be > added. Conor Cleary has updated the pull request incrementally with one additional commit since the last revision: 8283544: Removed testing comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/8017/files - new: https://git.openjdk.java.net/jdk/pull/8017/files/5c1e3a72..cb02c76d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8017=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8017=00-01 Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8017.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8017/head:pull/8017 PR: https://git.openjdk.java.net/jdk/pull/8017
