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

x/net/http2: exceeding the maximum flow-control window should be a connection error, not a stream error #18259

Closed
tombergan opened this issue Dec 9, 2016 · 2 comments
Closed

x/net/http2: exceeding the maximum flow-control window should be a connection error, not a stream error #18259

tombergan opened this issue Dec 9, 2016 · 2 comments
Labels
FrozenDueToAge

Comments

@tombergan
Copy link
Contributor

We have:
https://github.com/golang/net/blob/c46f265c325130a7a6c7b27db8c6fe14b64f1a68/http2/server.go#L1185

if !st.flow.add(int32(f.Increment)) {
  return streamError(f.StreamID, ErrCodeFlowControl)
}

But the spec says:
http://httpwg.org/specs/rfc7540.html#InitialWindowSize

An endpoint MUST treat a change to SETTINGS_INITIAL_WINDOW_SIZE that causes any flow-control window to exceed the maximum size as a connection error (Section 5.4.1) of type FLOW_CONTROL_ERROR.
@bradfitz
Copy link
Contributor

bradfitz commented Dec 9, 2016

Sure? The code you cite is from processing WINDOW_UPDATE frames, but the spec you cite is about SETTINGS frames.

There are at least parts of the spec that suggest FLOW_CONTROL_ERROR can be used as both connection and stream errors. ("A receiver MAY respond with a stream error (Section 5.4.2) or connection error (Section 5.4.1) of type FLOW_CONTROL_ERROR if it is unable to accept a frame.")

@tombergan
Copy link
Contributor Author

You're completely right, I have no idea what I was thinking.

@golang golang locked and limited conversation to collaborators Dec 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bradfitz @gopherbot @tombergan