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: handle Trailer headers according to RFC 7230 #23908

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

Comments

Projects
None yet
4 participants
@urld
Copy link
Contributor

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.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2018

@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Mar 28, 2018

@bradfitz

This comment has been minimized.

Copy link
Member

commented Mar 28, 2018

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.

@gopherbot

This comment has been minimized.

Copy link

commented Apr 2, 2018

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

@gopherbot

This comment has been minimized.

Copy link

commented Apr 2, 2018

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

http2, http/httpguts: move ValidTrailerHeader to new common package h…
…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 gopherbot closed this in ea3f329 Apr 16, 2018

@gopherbot

This comment has been minimized.

Copy link

commented May 5, 2018

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

net/http: update bundled http2
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

This comment has been minimized.

Copy link

commented May 7, 2018

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

lex/httplex, http/httpguts: merge the httplex package into httpguts
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

This comment has been minimized.

Copy link

commented May 12, 2018

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

devapp/owners: remove net/lex
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>

@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.
You can’t perform that action at this time.