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

net/http: can't set Transfer-Encoding explicitly on outgoing Request #28026

Open
johnSchnake opened this Issue Oct 4, 2018 · 4 comments

Comments

Projects
None yet
5 participants
@johnSchnake
Copy link

commented Oct 4, 2018

httputil.DumpRequestOut does not currently print anything w.r.t. Transfer-Encoding (TE) even though the information is printed via DumpRequest. In addition, there is special logic for handling TE that seems like maybe it hasn't been extracted out so the logic is hard-coded into DumpRequest which means that it could drift from what is actually done when sending the request.

I've not spent too much time looking at possible solutions but the Transfer-Encoding definitely seems to be a special case and its not particularly intuitive whats going on to your request if you set the header field manually or if you set the request.TransferEncoding.

The docs say that DumpRequest is for server side debugging whereas DumpRequestOut is for clientside, but I'm not familiar with why they couldn't/shouldn't be more similar (or the same).

Not sure if this logic should be put into DumpRequestOut, if not, why? And I'd like to get a bit of feedback if this is a known issue that maintainers would like to have cleaned up.

What version of Go are you using (go version)?

Go playground, any go version as far as I'm aware.

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

Just using the playground as an example.

What did you do?

https://play.golang.org/p/vW7oNw0Jjz6

What did you expect to see?

The same information for Transfer-Encoding available on both Dump... outputs.

What did you see instead?

Transfer-Encoding is printed during DumpRequest but not DumpRequestOut

@bradfitz

This comment has been minimized.

Copy link
Member

commented Oct 5, 2018

DumpRequestOut is implemented with the actual http.Transport writing to a buffer.

Are you actually seeing it appear on the wire but not in DumpRequestOut?

@johnSchnake

This comment has been minimized.

Copy link
Author

commented Oct 5, 2018

So I had thought that this data was being sent since I was able to send the request to a test server, respond with it back, and the TransferEncoding was still set appropriately. However I've now used wireshark to check and it is not actually sent in the request.

https://golang.org/src/net/http/transfer.go?h=writeHeader#L107 seems to show that the transferWriter only sets its transferEncoding in a very specific situation so won't generally copy the requests TransferEncoding value at all.

https://play.golang.org/p/pM6PmzpsaX7 is a minimal example which has one line commented out which, if toggled, changes the output.

Transfer encoding set (and only to chunked) when:

  • there is a body
  • its a type of request that normally might have a body (POST, PUT, etc
  • content length was set to < 0
  • we do not set any TransferEncoding value

That means there is no way to ever actually set any other TransferEncoding and if you try to manually set it to chunked it actually prevents the message from being sent with the chunked transfer encoding.

Is that the intended behavior? I've never actually wanted to change the TE but I was developing a tool to modify/send requests and assumed that someone might need to change that field. Maybe that is just an unnecessary concern?

@johnSchnake

This comment has been minimized.

Copy link
Author

commented Dec 7, 2018

/remove WaitingForInfo

@agnivade agnivade removed the WaitingForInfo label Dec 8, 2018

@bcmills bcmills changed the title httputil.DumpRequest out doesn't print Transfer-Encoding net/http: can't set Transfer-Encoding explicitly on outgoing Request Dec 20, 2018

@bcmills bcmills added this to the Go1.13 milestone Dec 20, 2018

@bcmills

This comment has been minimized.

Copy link
Member

commented Dec 20, 2018

Is that the intended behavior?

Back to @bradfitz to decide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.