[net] http2: prevent processing DATA before HEADERS
Michael Fraenkel (Gerrit)
Michael Fraenkel has uploaded this change for review.
http2: prevent processing DATA before HEADERS
DATA frames must only be processed after all HEADER frames have been
processed first.
Fixes golang/go#43965
Change-Id: I1607c713cc766c6e48c9abca8ca1671cf0575167
---
M http2/transport.go
M http2/transport_test.go
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/http2/transport.go b/http2/transport.go
index b97adff..c746f2f 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -2220,7 +2220,7 @@
}
return nil
}
- if !cs.firstByte {
+ if !cs.pastHeaders {
cc.logf("protocol error: received DATA before a HEADERS frame")
rl.endStreamError(cs, StreamError{
StreamID: f.StreamID,
diff --git a/http2/transport_test.go b/http2/transport_test.go
index 750813b..ed6cc31 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -3787,6 +3787,14 @@
if err, ok := err.(StreamError); !ok || err.Code != ErrCodeProtocol {
return fmt.Errorf("expected stream PROTOCOL_ERROR, got: %v", err)
}
+ // Third request returns a DATA frame after HEADERS with a 100 status code.
+ resp, err = ct.tr.RoundTrip(req)
+ if err == nil {
+ return fmt.Errorf("RoundTrip expected error, got response: %+v", resp)
+ }
+ if err, ok := err.(StreamError); !ok || err.Code != ErrCodeProtocol {
+ return fmt.Errorf("expected stream PROTOCOL_ERROR, got: %v", err)
+ }
return nil
}
ct.server = func() error {
@@ -3815,6 +3823,17 @@
})
case 3:
ct.fr.WriteData(f.StreamID, true, []byte("payload"))
+ case 5:
+ // Send a 100 continue followed by DATA
+ var buf bytes.Buffer
+ enc := hpack.NewEncoder(&buf)
+ enc.WriteField(hpack.HeaderField{Name: ":status", Value: "100"})
+ ct.fr.WriteHeaders(HeadersFrameParam{
+ StreamID: f.StreamID,
+ EndHeaders: true,
+ BlockFragment: buf.Bytes(),
+ })
+ ct.fr.WriteData(f.StreamID, true, []byte("payload"))
}
default:
return fmt.Errorf("Unexpected client frame %v", f)
To view, visit change 330415. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Attention is currently required from: Tom Bergan, Michael Fraenkel.
Patch set 1:Code-Review +2
Dmitri Shuralyov (Gerrit)
Attention is currently required from: Tom Bergan, Michael Fraenkel.
Patch set 1:Run-TryBot +1Trust +1
1 comment:
Commit Message:
Patch Set #1, Line 12:
Fixes golang/go#43965That issue is currently for the net/http package in the main Go repo, but this CL is for the x/net repo.
If this is merged as is, that issue will need to be re-opened to track this x/net/http2 fix being vendored (or bundled) into net/http. Alternatively, you can replace "Fixes" with "For", or create a new issue for x/net/http2 specifically.
To view, visit change 330415. To unsubscribe, or for help writing mail filters, visit settings.
Michael Fraenkel (Gerrit)
Attention is currently required from: Tom Bergan, Dmitri Shuralyov.
1 comment:
Commit Message:
Patch Set #1, Line 12:
Fixes golang/go#43965
That issue is currently for the net/http package in the main Go repo, but this CL is for the x/net r […]
Is there a plan to refresh the http2 bundle for 1.17 before its released?
To view, visit change 330415. To unsubscribe, or for help writing mail filters, visit settings.
Michael Fraenkel (Gerrit)
Commit Message:
Patch Set #1, Line 12:
Fixes golang/go#43965
Is there a plan to refresh the http2 bundle for 1. […]
Ack
To view, visit change 330415. To unsubscribe, or for help writing mail filters, visit settings.
Michael Fraenkel (Gerrit)
Attention is currently required from: Tom Bergan, Dmitri Shuralyov.
Michael Fraenkel uploaded patch set #2 to this change.
http2: prevent processing DATA before HEADERS
DATA frames must only be processed after all HEADER frames have been
processed first.
For golang/go#43965
Change-Id: I1607c713cc766c6e48c9abca8ca1671cf0575167
---
M http2/transport.go
M http2/transport_test.go
2 files changed, 20 insertions(+), 1 deletion(-)
To view, visit change 330415. To unsubscribe, or for help writing mail filters, visit settings.
Dmitri Shuralyov (Gerrit)
Attention is currently required from: Tom Bergan, Michael Fraenkel.
Patch set 2:Trust +1
1 comment:
Commit Message:
Patch Set #1, Line 12:
Fixes golang/go#43965
Ack
If the net/http issue fix is intended for 1.17, the Go repo will need to be updated after this is submitted. If it’s meant for 1.18, this should wait until the 1.17 is out and its code freeze is over. (The code freeze applies to the main repo and all packages that are vendored/bundled into it.)
To view, visit change 330415. To unsubscribe, or for help writing mail filters, visit settings.
Michael Fraenkel (Gerrit)
Attention is currently required from: Tom Bergan.
1 comment:
Patchset:
PTAL
To view, visit change 330415. To unsubscribe, or for help writing mail filters, visit settings.
Michael Fraenkel (Gerrit)
Attention is currently required from: Tom Bergan.
Michael Fraenkel uploaded patch set #3 to this change.
http2: prevent processing DATA before HEADERS
DATA frames must only be processed after all HEADER frames have been
processed first.
For golang/go#43965
Change-Id: I1607c713cc766c6e48c9abca8ca1671cf0575167
---
M http2/transport_test.go
M http2/transport.go
2 files changed, 34 insertions(+), 1 deletion(-)
To view, visit change 330415. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Attention is currently required from: Michael Fraenkel, Tom Bergan.
Patch set 3:Run-TryBot +1Code-Review +2
1 comment:
Patchset:
I'm not sure what happened to this CL. It had a positive trybot result and sufficient plusses at some point in the past, but now it's missing the trybot result and I think we changed the way trust labels work.
Sorry for letting this fall through the cracks.
To view, visit change 330415. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Attention is currently required from: Michael Fraenkel, Tom Bergan.
1 comment:
Patchset:
11 of 11 TryBots failed. […]
Looks like this needs to be resynced.
To view, visit change 330415. To unsubscribe, or for help writing mail filters, visit settings.
Dmitri Shuralyov (Gerrit)
Attention is currently required from: Damien Neil, Michael Fraenkel, Tom Bergan.
Patch set 3:Code-Review +1