[XrdHttp] Add back parsing of Transfer-Encoding header

[CohenQU]

This commit reintroduces a header parser for Transfer-Encoding: chunked to maintain compatibility with the EGI Nagios probe. This issue was first mentioned in #2058.

Fixes: d96b2b70487e159b47c24925abcfa00f975ec3d6, #2009

[olifre]

Thanks for the patch! As already mentioned in the linked issue, I can confirm that this fixes the EGI probe (and other use cases using Transfer-Encoding: chunked) in my testing.
It still feels a bit surprising that setting the status header also toggles the transfer-encoding, so I would be curious as to which clients require this workaround (maybe it is also possible to fix these in their corresponding upstreams instead).

[CohenQU]

@olifre
One example is the osdf-client, where we create the HTTP request by using net/http package (line 453). However, the workaround seems a necessary step for all clients implemented with golang, until there is any update on their end.

The issue we encountered before is as follows. In Golang, we can set headers in HTTP GET requests through:

req.HTTPRequest.Header.Set("X-Transfer-Status", "true")
req.HTTPRequest.Header.Set("TE", "trailers")
req.HTTPRequest.Header.Set("Transfer-Encoding", "chunked")

However, the XRootD server is only able to capture the X-Transfer-Status and TE headers when parsing the headers, while the Transfer-Encoding header is missing. Our analysis suggests that the Go HTTP client is automatically handling the chunking of the request body, which result in the removal of the Transfer-Encoding header from the outgoing request. And this is why we came up with the workaround we discussed before.

[ccaffy]

All good for me.

[olifre]

One example is the osdf-client, where we create the HTTP request by using net/http package (line 453). However, the workaround seems a necessary step for all clients implemented with golang, until there is any update on their end.

Thanks for the details!

I did not find the actual bug in osdf-client yet, since my Go knowledge is limited, but using and modifying a simple example such as https://gist.github.com/yinchunxiang/ef170ce11927a2f057a32870e2dd4cd2 yields a working request with the headers:

POST / HTTP/1.1
Host: localhost:9999
User-Agent: Go-http-client/1.1
Transfer-Encoding: chunked
Content-Type: application/json
Te: trailers
X-Transfer-Status: true

So this is not a general bug in net/http, but likely a problem in the way it is used by osdf-client (maybe since the HTTP version is not fixed to 1.1, and Transfer-Encoding: chunked is forbidden in HTTP/2, or maybe the Content-Length header is set accidentally?).

In any case, since osdf-client seems to be the only problematic client, we could move this part of the discussion over to osdf-client if you like — it seems it's a bug in that client which could be fixed (and subsequently, the client-specific workaround in XRootD could be dropped?).

So the TL;DR would be: For sure the fix in this issue should go in, but maybe the workaround which is still in can also be removed once the problematic client is fixed ;-).

[CohenQU]

@olifre I concur with your recommendations. I will initiate a dialogue with the developers on the osdf-client side to address the issue. Once we have implemented the necessary changes, I will then proceed to establish an additional PR here. Thank you for your invaluable input.

[olifre]

@CohenQU Thanks a lot to you, too! Since this will also make osdf-client compatible with other servers (even though likely XRootD is the only server used there, if I understand it correctly), fixing this on the client side is for sure also good for compatibility in general.

[amadio]

@CohenQU I think the commit message could be improved, but the patch itself looks good to go. I'd use [XrdHttp] Add back parsing of Transfer-Encoding header or something like that, and also a reference to the commit that removed it with Fixes: <commit hash>, <issue#>. Thanks!

[CohenQU]

@amadio The commit message is updated.

[amadio]

Only the title of the pull request has been updated, the commit message remains the same.

[CohenQU]

@amadio Sorry, the update on my local branch didn't seem to deploy well, but it should be updated now.

[amadio]

Thank you! I am merging, but please note that the issue will only close when I merge the devel branch into master, you do not need to close that by hand.