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

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

Comments

@tombergan
Copy link
Contributor

@tombergan tombergan commented Dec 9, 2016

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

This comment has been minimized.

Copy link
Contributor

@bradfitz 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

This comment has been minimized.

Copy link
Contributor Author

@tombergan tombergan commented Dec 9, 2016

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

@tombergan tombergan closed this Dec 9, 2016
@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.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.