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

http2: return PROTOCOL_ERROR when http2 reads an http1.1 response #58

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jingyugao
Copy link

@jingyugao jingyugao commented Nov 20, 2019

When destinate is "http/1.1 " server, client should return "PROTOCOL_ERROR".
Before this pr,client will return an frame will bad length. and then cause an "too large frame" error.
see golang/go#35724

@gopherbot
Copy link
Contributor

This PR (HEAD: 8eaf1c0) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/net/+/207851 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@bradfitz bradfitz changed the title return "PROTOCOL_ERROR" when http2 connect http1.1 http2: return PROTOCOL_ERROR when http2 reads an http1.1 response Nov 20, 2019
@gopherbot
Copy link
Contributor

Message from Brad Fitzpatrick:

Patch Set 1:

This is missing background information about when this would happen & why we'd want this.

It's also missing tests, but you can wait on those until we understand what this change is about.

Also, see https://github.com/golang/go/wiki/CommitMessage


Please don’t reply on this GitHub thread. Visit golang.org/cl/207851.
After addressing review feedback, remember to publish your drafts!

@jingyugao jingyugao closed this Nov 20, 2019
@jingyugao jingyugao reopened this Nov 20, 2019
@gopherbot
Copy link
Contributor

Message from Jingyu Gao:

Patch Set 2:

https://tools.ietf.org/html/rfc7540#section-6.5
according to the rfc, we should return a PROTOCOL_ERROR when receive a badly frame.
But now, we just treat it is a normal frame and after parse it, we get an FRAME_SIZE_ERROR.
I think we should distinguish the error.


Please don’t reply on this GitHub thread. Visit golang.org/cl/207851.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jingyu Gao:

Patch Set 2:

Maybe we can check the first frame that client receved is euqal []byte{0 0 6 4 0 0 0 0 0 0 5 0 0 64 0} ?


Please don’t reply on this GitHub thread. Visit golang.org/cl/207851.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: b16bfe7) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/net/+/207851 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Brad Fitzpatrick:

Patch Set 3:

Please file a bug on GitHub first and tell us how you got into this situation.


Please don’t reply on this GitHub thread. Visit golang.org/cl/207851.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 4634c64) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/net/+/207851 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Jingyu Gao:

Patch Set 4:

I have add the test case in the pr.
PTAL.


Please don’t reply on this GitHub thread. Visit golang.org/cl/207851.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Brad Fitzpatrick:

Patch Set 4:

Patch Set 4:

I have add the test case in the pr.
PTAL.

I won't be looking at the code until there's a bug with background info about the situation where this becomes necessary.

See my comments above.


Please don’t reply on this GitHub thread. Visit golang.org/cl/207851.
After addressing review feedback, remember to publish your drafts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants