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

[Breaking Change Request] [io/http] Drop Content-Length:0 header from CONNECT,DELETE,GET,HEAD requests #45871

Closed
aam opened this issue Apr 29, 2021 · 11 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. breaking-change-request This tracks requests for feedback on breaking changes library-io

Comments

@aam
Copy link
Contributor

aam commented Apr 29, 2021

Summary

We propose to not to include Content-Length header in CONNECT,DELETE,GET,HEAD http requests when its value is 0(when there is no body as part of http request).

Rationale

Presence of Content-Length:0 header runs against guidelines provided in RFC-7320 https://tools.ietf.org/html/rfc7230#section-3.3.2, which states that

A user agent SHOULD NOT send a
   Content-Length header field when the request message does not contain
   a payload body and the method semantics do not anticipate such a body.

The fact that dart:io HttpClientRequest code includes Content-Length:0 header in GET requests was recently found to be a culprit of inability for Dart http client code to talk to Google Cloud Run websocket services (see #45139). So this change will fix that problem.

See https://dart-review.googlesource.com/c/sdk/+/194881 for the actual change.

@aam aam added area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. library-io breaking-change-request This tracks requests for feedback on breaking changes labels Apr 29, 2021
@aam
Copy link
Contributor Author

aam commented Apr 30, 2021

@franklinyow, just wanted to let you know that announcement email has been sent out (https://groups.google.com/a/dartlang.org/g/announce/c/b0cncgjcRf4)

@rspilker
Copy link
Contributor

What about other methods that also don't have defined semantics for a payload, like DELETE?

@aam
Copy link
Contributor Author

aam commented Apr 30, 2021

@rspilker wrote

What about other methods that also don't have defined semantics for a payload, like DELETE?

Good question, thanks for pointing that out.
Per https://tools.ietf.org/html/rfc7231 It looks like DELETE and CONNECT in addition to GET and HEAD have similar language that "A payload within a ... request message has no defined semantics;
sending a payload body on a HEAD request might cause some existing
implementations to reject the request."
I believe we should extend proposed change to all four.

@aam aam changed the title [Breaking Change Request] [io/http] Drop Content-Length:0 header from GET/HEAD requests [Breaking Change Request] [io/http] Drop Content-Length:0 header from CONNECT,DELETE,GET,HEAD requests May 1, 2021
@aam
Copy link
Contributor Author

aam commented May 12, 2021

any updates on this @franklinyow ?

@franklinyow
Copy link
Contributor

Ping @vsmenon @matanlurey @Hixie for approval

@Hixie
Copy link
Contributor

Hixie commented May 14, 2021

Following the standards SGTM.

@vsmenon
Copy link
Member

vsmenon commented May 24, 2021

lgtm

@franklinyow
Copy link
Contributor

Approved

@daniel-mf
Copy link

Any updates?
Really would like to have this fixed so I could remove the massive workaround in my project.
Thanks!

@thomas-ferchau
Copy link

If I read the information in the commit correctly, the change is included in the dev releases since 2.14.0-181.0.dev

@aam
Copy link
Contributor Author

aam commented Jun 14, 2021

Fixed in 6c254fa

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. breaking-change-request This tracks requests for feedback on breaking changes library-io
Projects
None yet
Development

No branches or pull requests

7 participants