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, x/net/http2: update RFC numbers used as normative references #21974

Closed
mikioh opened this issue Sep 22, 2017 · 9 comments

Comments

Projects
None yet
4 participants
@mikioh
Copy link
Contributor

commented Sep 22, 2017

The documentation in x/net/http and x/net/http2 packages still contains the obsolete RFC 2616 as a normative reference. I think it's better to replace the reference to RFC 2616 with a series of RFCs 7230 through 7235 for avoiding unnecessary confusion.

When we see some inconsistency, for example, RFC 2616 vs. RFC 723x, in the current implementation, that should be addressed as another issue. So this should be a documentation issue.

@urld

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2018

Is there a convention for referring to RFCs?

I see a mix of mostly just mentions of section numbers in the comments and sometimes URLs to specific section numbers of RFC 2616 (exclusively via www.w3.org).

In my opinion the URL makes sense if it is shown by godoc which seems to be the case in golang.org/x/net/lex/httplex.

@bradfitz bradfitz added this to the Unplanned milestone Jan 30, 2018

@urld

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2018

Here are the inconsistencies with RFC-7230 i have found in net/http so far:

The handling of "hop-by-hop" Headers is defined differently:

Exactly when "close" connection options have to be sent has been
clarified. Also, "hop-by-hop" header fields are required to appear
in the Connection header field; just because they're defined as hop-
by-hop in this specification doesn't exempt them. (Section 6.1)

However the current behavior has to be kept to maintain compatibility.

Other than that RFC 7230 defines more forbidden trailer headers for chunked transfers (see section 4.1.2) than currently implemented.

Note that i only looked in the places where rfc 2616 was referenced explicitly , so its hard to tell if there are other non obvious inconsistencies.

@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2018

Is there a convention for referring to RFCs?

There's no convention, but the document should look like a document, not a side note or writing to the back of a paper napkin. I'm fine with either RFC NNNN, the section X of RFC NNN or URL.

@gopherbot

This comment has been minimized.

Copy link

commented Feb 14, 2018

Change https://golang.org/cl/94095 mentions this issue: net/http: use RFC 723x as normative reference in docs

@urld

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2018

I uploaded a first patch with the documentation changes. It includes todos for inconsistencies.
However, i think these inconsistencies should be cleared up first, before the documentation change is submitted.

@mikioh

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2018

Thanks for tackling this issue.

these inconsistencies should be cleared up first

Please open a new issue for shooting such inconsistencies, for getting more eyeballs. I think that leaving TODOs and issues is fine for now because the transition from H1 to H2 is a bit complicated work and probably takes a bit long time.

@gopherbot

This comment has been minimized.

Copy link

commented Feb 15, 2018

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

gopherbot pushed a commit to golang/net that referenced this issue Feb 15, 2018

http2: use RFC 723x as normative reference in docs
Replace references to the obsoleted RFC 2616 with references to RFC
7230 through 7235, to avoid unnecessary confusion.

Updates golang/go#21974

Change-Id: Idbe3e73199f0bef9dbdbe1b75c39382799cd646c
Reviewed-on: https://go-review.googlesource.com/94555
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@urld

This comment has been minimized.

Copy link
Contributor

commented Feb 18, 2018

I have created an issue for the incorrect handling of Trailer headers : #23908

The handling of relative Location URIs in Redirect responses seem to be okay, but could use some addittional tests to ensure that fragments ar not thrown away. However im not sure if path.Clean should be called for absolute urls too. I will open another issue for discussion (or at least for additional tests) on this too. (edit: issue created: #23961)

gopherbot pushed a commit that referenced this issue Feb 20, 2018

net/http: use RFC 723x as normative reference in docs
Replace references to the obsoleted RFC 2616 with references to RFC
7230 through 7235, to avoid unnecessary confusion.
Obvious inconsistencies are marked with todo comments.

Updates #21974

Change-Id: I8fb4fcdd1333fc5193b93a2f09598f18c45e7a00
Reviewed-on: https://go-review.googlesource.com/94095
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@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 gopherbot closed this in 260ae19 May 6, 2018

@golang golang locked and limited conversation to collaborators May 6, 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.