diff --git a/http3/frames.go b/http3/frames.go index 9e1051af..4b46fb1b 100644 --- a/http3/frames.go +++ b/http3/frames.go @@ -104,30 +104,54 @@ func (p *frameParser) ParseNext(qlogger qlogwriter.Recorder) (frame, error) { }, nil case 0x4: // SETTINGS return parseSettingsFrame(r, l, p.streamID, qlogger) - case 0x3: // CANCEL_PUSH - case 0x5: // PUSH_PROMISE - case 0x7: // GOAWAY - f, err := parseGoAwayFrame(r, l) - if err != nil { - return nil, err - } + case 0x3: // unsupported: CANCEL_PUSH if qlogger != nil { qlogger.RecordEvent(qlog.FrameParsed{ StreamID: p.streamID, - Raw: qlog.RawInfo{ - Length: r.NumRead, - PayloadLength: int(l), - }, - Frame: qlog.Frame{Frame: qlog.GoAwayFrame{StreamID: f.StreamID}}, + Raw: qlog.RawInfo{Length: r.NumRead, PayloadLength: int(l)}, + Frame: qlog.Frame{Frame: qlog.CancelPushFrame{}}, + }) + } + case 0x5: // unsupported: PUSH_PROMISE + if qlogger != nil { + qlogger.RecordEvent(qlog.FrameParsed{ + StreamID: p.streamID, + Raw: qlog.RawInfo{Length: r.NumRead, PayloadLength: int(l)}, + Frame: qlog.Frame{Frame: qlog.PushPromiseFrame{}}, + }) + } + case 0x7: // GOAWAY + return parseGoAwayFrame(r, l, p.streamID, qlogger) + case 0xd: // unsupported: MAX_PUSH_ID + if qlogger != nil { + qlogger.RecordEvent(qlog.FrameParsed{ + StreamID: p.streamID, + Raw: qlog.RawInfo{Length: r.NumRead, PayloadLength: int(l)}, + Frame: qlog.Frame{Frame: qlog.MaxPushIDFrame{}}, + }) + } + case 0x2, 0x6, 0x8, 0x9: // reserved frame types + if qlogger != nil { + qlogger.RecordEvent(qlog.FrameParsed{ + StreamID: p.streamID, + Raw: qlog.RawInfo{Length: r.NumRead + int(l), PayloadLength: int(l)}, + Frame: qlog.Frame{Frame: qlog.ReservedFrame{Type: t}}, }) } - return f, nil - case 0xd: // MAX_PUSH_ID - case 0x2, 0x6, 0x8, 0x9: p.closeConn(quic.ApplicationErrorCode(ErrCodeFrameUnexpected), "") return nil, fmt.Errorf("http3: reserved frame type: %d", t) + default: + // unknown frame types + if qlogger != nil { + qlogger.RecordEvent(qlog.FrameParsed{ + StreamID: p.streamID, + Raw: qlog.RawInfo{Length: r.NumRead, PayloadLength: int(l)}, + Frame: qlog.Frame{Frame: qlog.UnknownFrame{Type: t}}, + }) + } } - // skip over unknown frames + + // skip over the payload if _, err := io.CopyN(io.Discard, r, int64(l)); err != nil { return nil, err } @@ -279,7 +303,7 @@ type goAwayFrame struct { StreamID quic.StreamID } -func parseGoAwayFrame(r *countingByteReader, l uint64) (*goAwayFrame, error) { +func parseGoAwayFrame(r *countingByteReader, l uint64, streamID quic.StreamID, qlogger qlogwriter.Recorder) (*goAwayFrame, error) { frame := &goAwayFrame{} startLen := r.NumRead id, err := quicvarint.Read(r) @@ -290,6 +314,13 @@ func parseGoAwayFrame(r *countingByteReader, l uint64) (*goAwayFrame, error) { return nil, errors.New("GOAWAY frame: inconsistent length") } frame.StreamID = quic.StreamID(id) + if qlogger != nil { + qlogger.RecordEvent(qlog.FrameParsed{ + StreamID: streamID, + Raw: qlog.RawInfo{Length: r.NumRead, PayloadLength: int(l)}, + Frame: qlog.Frame{Frame: qlog.GoAwayFrame{StreamID: frame.StreamID}}, + }) + } return frame, nil } diff --git a/http3/frames_test.go b/http3/frames_test.go index c84a095c..ee750180 100644 --- a/http3/frames_test.go +++ b/http3/frames_test.go @@ -9,7 +9,10 @@ import ( "time" "github.com/quic-go/quic-go" + "github.com/quic-go/quic-go/http3/qlog" + "github.com/quic-go/quic-go/qlogwriter" "github.com/quic-go/quic-go/quicvarint" + "github.com/quic-go/quic-go/testutils/events" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -30,17 +33,19 @@ func testFrameParserEOF(t *testing.T, data []byte) { func TestParserReservedFrameType(t *testing.T) { for _, ft := range []uint64{0x2, 0x6, 0x8, 0x9} { t.Run(fmt.Sprintf("type %#x", ft), func(t *testing.T) { - client, server := newConnPair(t) + var eventRecorder events.Recorder + client, server := newConnPairWithDatagrams(t, nil, &eventRecorder) data := quicvarint.Append(nil, ft) data = quicvarint.Append(data, 6) data = append(data, []byte("foobar")...) fp := frameParser{ + streamID: 42, r: bytes.NewReader(data), closeConn: client.CloseWithError, } - _, err := fp.ParseNext(nil) + _, err := fp.ParseNext(&eventRecorder) require.Error(t, err) require.ErrorContains(t, err, "http3: reserved frame type") @@ -53,6 +58,17 @@ func TestParserReservedFrameType(t *testing.T) { case <-time.After(time.Second): t.Fatal("timeout") } + + require.Equal(t, + []qlogwriter.Event{ + qlog.FrameParsed{ + StreamID: 42, + Raw: qlog.RawInfo{Length: len(data), PayloadLength: 6}, + Frame: qlog.Frame{Frame: qlog.ReservedFrame{Type: ft}}, + }, + }, + eventRecorder.Events(qlog.FrameParsed{}), + ) }) } } @@ -81,6 +97,60 @@ func TestParserUnknownFrameType(t *testing.T) { require.Equal(t, []byte("foo"), payload) } +func TestParserUnsupportedFrameTypes(t *testing.T) { + for _, tc := range []struct { + name string + ft uint64 + qf any + }{ + {name: "CANCEL_PUSH", ft: 0x3, qf: qlog.CancelPushFrame{}}, + {name: "PUSH_PROMISE", ft: 0x5, qf: qlog.PushPromiseFrame{}}, + {name: "MAX_PUSH_ID", ft: 0xd, qf: qlog.MaxPushIDFrame{}}, + } { + t.Run(tc.name, func(t *testing.T) { + var eventRecorder events.Recorder + + data := quicvarint.Append(nil, tc.ft) + data = quicvarint.Append(data, 6) + data = append(data, []byte("foobar")...) + df := &dataFrame{Length: 3} + data = df.Append(data) + data = append(data, []byte("foo")...) + + r := bytes.NewReader(data) + fp := frameParser{streamID: 42, r: r} + + f, err := fp.ParseNext(&eventRecorder) + require.NoError(t, err) + require.IsType(t, &dataFrame{}, f) + df = f.(*dataFrame) + require.Equal(t, uint64(3), df.Length) + payload := make([]byte, 3) + _, err = io.ReadFull(r, payload) + require.NoError(t, err) + require.Equal(t, []byte("foo"), payload) + + headerLen := quicvarint.Len(tc.ft) + quicvarint.Len(6) + dfLen, _ := expectedFrameLength(t, df) + require.Equal(t, + []qlogwriter.Event{ + qlog.FrameParsed{ + StreamID: 42, + Raw: qlog.RawInfo{Length: headerLen, PayloadLength: 6}, + Frame: qlog.Frame{Frame: tc.qf}, + }, + qlog.FrameParsed{ + StreamID: 42, + Raw: qlog.RawInfo{Length: dfLen, PayloadLength: 3}, + Frame: qlog.Frame{Frame: qlog.DataFrame{}}, + }, + }, + eventRecorder.Events(qlog.FrameParsed{}), + ) + }) + } +} + func TestParserHeadersFrame(t *testing.T) { data := quicvarint.Append(nil, 1) // type byte data = quicvarint.Append(data, 0x1337) diff --git a/http3/qlog/frame.go b/http3/qlog/frame.go index d2428900..78959a93 100644 --- a/http3/qlog/frame.go +++ b/http3/qlog/frame.go @@ -20,6 +20,16 @@ func (f Frame) encode(enc *jsontext.Encoder) error { return frame.encode(enc) case SettingsFrame: return frame.encode(enc) + case PushPromiseFrame: + return frame.encode(enc) + case CancelPushFrame: + return frame.encode(enc) + case MaxPushIDFrame: + return frame.encode(enc) + case ReservedFrame: + return frame.encode(enc) + case UnknownFrame: + return frame.encode(enc) } // This shouldn't happen if the code is correctly logging frames. // Write a null token to produce valid JSON. @@ -131,3 +141,71 @@ func (f *SettingsFrame) encode(enc *jsontext.Encoder) error { h.WriteToken(jsontext.EndObject) return h.err } + +// A PushPromiseFrame is a PUSH_PROMISE frame +type PushPromiseFrame struct{} + +func (f *PushPromiseFrame) encode(enc *jsontext.Encoder) error { + h := encoderHelper{enc: enc} + h.WriteToken(jsontext.BeginObject) + h.WriteToken(jsontext.String("frame_type")) + h.WriteToken(jsontext.String("push_promise")) + h.WriteToken(jsontext.EndObject) + return h.err +} + +// A CancelPushFrame is a CANCEL_PUSH frame +type CancelPushFrame struct{} + +func (f *CancelPushFrame) encode(enc *jsontext.Encoder) error { + h := encoderHelper{enc: enc} + h.WriteToken(jsontext.BeginObject) + h.WriteToken(jsontext.String("frame_type")) + h.WriteToken(jsontext.String("cancel_push")) + h.WriteToken(jsontext.EndObject) + return h.err +} + +// A MaxPushIDFrame is a MAX_PUSH_ID frame +type MaxPushIDFrame struct{} + +func (f *MaxPushIDFrame) encode(enc *jsontext.Encoder) error { + h := encoderHelper{enc: enc} + h.WriteToken(jsontext.BeginObject) + h.WriteToken(jsontext.String("frame_type")) + h.WriteToken(jsontext.String("max_push_id")) + h.WriteToken(jsontext.EndObject) + return h.err +} + +// A ReservedFrame is one of the reserved frame types +type ReservedFrame struct { + Type uint64 +} + +func (f *ReservedFrame) encode(enc *jsontext.Encoder) error { + h := encoderHelper{enc: enc} + h.WriteToken(jsontext.BeginObject) + h.WriteToken(jsontext.String("frame_type")) + h.WriteToken(jsontext.String("reserved")) + h.WriteToken(jsontext.String("frame_type_bytes")) + h.WriteToken(jsontext.Uint(f.Type)) + h.WriteToken(jsontext.EndObject) + return h.err +} + +// An UnknownFrame is an unknown frame type +type UnknownFrame struct { + Type uint64 +} + +func (f *UnknownFrame) encode(enc *jsontext.Encoder) error { + h := encoderHelper{enc: enc} + h.WriteToken(jsontext.BeginObject) + h.WriteToken(jsontext.String("frame_type")) + h.WriteToken(jsontext.String("unknown")) + h.WriteToken(jsontext.String("frame_type_bytes")) + h.WriteToken(jsontext.Uint(f.Type)) + h.WriteToken(jsontext.EndObject) + return h.err +} diff --git a/http3/qlog/frame_test.go b/http3/qlog/frame_test.go index 872f87d2..ab1bfaa4 100644 --- a/http3/qlog/frame_test.go +++ b/http3/qlog/frame_test.go @@ -11,6 +11,8 @@ import ( ) func check(t *testing.T, f any, expected map[string]any) { + t.Helper() + var buf bytes.Buffer enc := jsontext.NewEncoder(&buf) require.NoError(t, (Frame{Frame: f}).encode(enc)) @@ -144,3 +146,35 @@ func TestSettingsFrame(t *testing.T) { }) } } + +func TestPushPromiseFrame(t *testing.T) { + check(t, PushPromiseFrame{}, map[string]any{ + "frame_type": "push_promise", + }) +} + +func TestCancelPushFrame(t *testing.T) { + check(t, CancelPushFrame{}, map[string]any{ + "frame_type": "cancel_push", + }) +} + +func TestMaxPushIDFrame(t *testing.T) { + check(t, MaxPushIDFrame{}, map[string]any{ + "frame_type": "max_push_id", + }) +} + +func TestReservedFrame(t *testing.T) { + check(t, ReservedFrame{Type: 0x1f}, map[string]any{ + "frame_type": "reserved", + "frame_type_bytes": 0x1f, + }) +} + +func TestUnknownFrame(t *testing.T) { + check(t, UnknownFrame{Type: 0x2a}, map[string]any{ + "frame_type": "unknown", + "frame_type_bytes": 0x2a, + }) +} diff --git a/http3/stream_test.go b/http3/stream_test.go index 43fecd35..9afd2c6e 100644 --- a/http3/stream_test.go +++ b/http3/stream_test.go @@ -99,7 +99,6 @@ func TestStreamReadDataFrames(t *testing.T) { buf.Write([]byte("invalid")) _, err = str.Read([]byte{0}) require.Error(t, err) - require.Empty(t, eventRecorder.Events(qlog.FrameParsed{})) } func TestStreamInvalidFrame(t *testing.T) {