Navigation Menu

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

Jump to bottom

net/http: handle Trailer headers according to RFC 7230 #23908

Closed
urld opened this issue Feb 18, 2018 · 7 comments
Closed

net/http: handle Trailer headers according to RFC 7230 #23908

urld opened this issue Feb 18, 2018 · 7 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.11

Comments

@urld
Copy link
Contributor

urld commented Feb 18, 2018

The method declareTrailer in net/http/server.go does not handle Trailer headers according to RFC 7230.

While the the obsoleted RFC 2616 section 14.40 states:

Message header fields listed in the Trailer header field MUST NOT
include the following header fields:

  • Transfer-Encoding
  • Content-Length
  • Trailer

declareTrailer only ignores the headers mentioned above, but RFC 7230 section 4.1.2 states:

A sender MUST NOT generate a trailer that contains a field necessary
for message framing (e.g., Transfer-Encoding and Content-Length),
routing (e.g., Host), request modifiers (e.g., controls and
conditionals in Section 5 of [RFC7231]), authentication (e.g., see
[RFC7235] and [RFC6265]), response control data (e.g., see Section
7.1 of [RFC7231]), or determining how to process the payload (e.g.,
Content-Encoding, Content-Type, Content-Range, and Trailer).

x/net/http2 already implements this behavior in ValidTrailerHeader. Maybe this method can be moved to net/http.
@urld urld mentioned this issue Feb 18, 2018
Closed
@serialseb serialseb mentioned this issue Feb 23, 2018
Closed
@ianlancetaylor
Copy link
Contributor

CC @bradfitz @tombergan

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 28, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Mar 28, 2018
@bradfitz
Copy link
Contributor

We don't want to export this function in net/http, but we can move it somewhere common in x/net and vendor it in, perhaps under https://godoc.org/golang.org/x/net/http somewhere alongside httpproxy. Maybe httpimpl or httpguts or httpdetails.

@bradfitz bradfitz added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Mar 28, 2018
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 28, 2018
@gopherbot
Copy link

Change https://golang.org/cl/104042 mentions this issue: http2: move ValidTrailerHeader to common package http/httpguts

@gopherbot
Copy link

Change https://golang.org/cl/104075 mentions this issue: net/http: omit forbidden Trailer headers from response

gopherbot pushed a commit to golang/net that referenced this issue Apr 16, 2018
@urld @bradfitz
http2, http/httpguts: move ValidTrailerHeader to new common package h…
a35a21d 
…ttp/httpguts

Introduce a common package x/net/http/httpguts which can be vendored by
net/http to share detail implementations of the HTTP specification with
x/net/http2.

Updates golang/go#23908

Change-Id: Id5a2d51e05135436cf406c4c4d1b13fca7f84a32
Reviewed-on: https://go-review.googlesource.com/104042
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/111655 mentions this issue: net/http: update bundled http2

gopherbot pushed a commit that referenced this issue May 6, 2018
@namusyaka @bradfitz
net/http: update bundled http2
260ae19 
Updates http2 to x/net/http2 git rev 5f9ae10 for:

    http2: terminate await request cancel goroutine on conn close
    https://golang.org/cl/108415

    http2: don't sniff Content-type in Server when X-Content-Type-Options:nosniff
    https://golang.org/cl/107295

    http2, http/httpguts: move ValidTrailerHeader to new common package http/httpguts
    https://golang.org/cl/104042

    all: remove "the" duplications
    https://golang.org/cl/94975

    http2: use RFC 723x as normative reference in docs
    https://golang.org/cl/94555

    all: use HTTPS for iana.org links
    https://golang.org/cl/89415

Fixes #24795
Fixes #24776
Updates #23908
Fixes #21974

Change-Id: I7985617a7dde56cc5ed8670d73b26f8307be83d6
Reviewed-on: https://go-review.googlesource.com/111655
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/111875 mentions this issue: lex/httplex, http/httpguts: merge the httplex package into httpguts

gopherbot pushed a commit to golang/net that referenced this issue May 7, 2018
@bradfitz
lex/httplex, http/httpguts: merge the httplex package into httpguts
cbb82b5 
httplex was the original package name for shared code between net/http
and x/net/http2, but its name was too specific, and http/httpguts was
added later for other shared code.

We discussed merging httplex into httpguts at the time, but it didn't
happen earlier. This finishes the move.

Updates golang/go#23908

Change-Id: Ic7d6f39e584ca579d34b5ef5ec6a0c002a38a83c
Reviewed-on: https://go-review.googlesource.com/111875
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/112996 mentions this issue: devapp/owners: remove net/lex

gopherbot pushed a commit to golang/build that referenced this issue May 13, 2018
@dmitshur @bradfitz
devapp/owners: remove net/lex
f691756 
net/lex has been merged into net/http in golang/net@cbb82b5,
and bradfitz is already the primary for that.

Updates golang/go#23908.

Change-Id: Icf5eca0751829e26d1951ec6c0eb2d22348695e9
Reviewed-on: https://go-review.googlesource.com/112996
Reviewed-by: Matt Layher <mdlayher@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Matt Layher <mdlayher@gmail.com>
@wangxf1987 wangxf1987 mentioned this issue May 23, 2018
Closed
@golang golang locked and limited conversation to collaborators May 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
4 participants
@bradfitz @ianlancetaylor @gopherbot @urld