Copilot commented on code in PR #13465: URL: https://github.com/apache/trafficserver/pull/13465#discussion_r3693407483
########## tests/gold_tests/qmux/go_qmux_client/main.go: ########## @@ -0,0 +1,340 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "bytes" + "context" + "crypto/tls" + "errors" + "flag" + "fmt" + "io" + "net" + "os" + "strconv" + "time" + + "github.com/okdaichi/qmux-go/qmux" + "github.com/quic-go/qpack" + "github.com/quic-go/quic-go/quicvarint" +) + +const ( + qmuxALPN = "h3qx-01" + bodyChunkSize = 8 * 1024 + largeBodySize = 300000 + + h3FrameData = 0x00 + h3FrameHeaders = 0x01 + h3FrameSettings = 0x04 + + h3ControlStream = 0x00 + h3QPACKEncoderStream = 0x02 + h3QPACKDecoderStream = 0x03 +) + +type requestCase struct { + name string + method string + path string + requestSize int + responseSize int +} + +func generatedBody(size int) []byte { + var body bytes.Buffer + for i := 0; body.Len() < size; i++ { + fmt.Fprintf(&body, "%07x ", i) + } + return body.Bytes()[:size] +} + +func writeVarInt(w io.Writer, value uint64) error { + encoded := quicvarint.Append(nil, value) + _, err := w.Write(encoded) + return err +} + +func writeFrame(w io.Writer, frameType uint64, payload []byte) error { + header := quicvarint.Append(nil, frameType) + header = quicvarint.Append(header, uint64(len(payload))) + if _, err := w.Write(header); err != nil { + return err + } + _, err := w.Write(payload) + return err +} + +func writeRequestBody(w io.Writer, body []byte) error { + for len(body) > 0 { + chunkSize := min(len(body), bodyChunkSize) + if err := writeFrame(w, h3FrameData, body[:chunkSize]); err != nil { + return err + } + body = body[chunkSize:] + } + return nil +} + +func openUniStream(ctx context.Context, conn *qmux.Conn, streamType uint64) error { + stream, err := conn.OpenUniStreamSync(ctx) + if err != nil { + return err + } + return writeVarInt(stream, streamType) +} + +func initializeHTTP3(ctx context.Context, conn *qmux.Conn) error { + control, err := conn.OpenUniStreamSync(ctx) + if err != nil { + return fmt.Errorf("open control stream: %w", err) + } + if err := writeVarInt(control, h3ControlStream); err != nil { + return fmt.Errorf("write control stream type: %w", err) + } + if err := writeFrame(control, h3FrameSettings, nil); err != nil { + return fmt.Errorf("write SETTINGS frame: %w", err) + } + + if err := openUniStream(ctx, conn, h3QPACKEncoderStream); err != nil { + return fmt.Errorf("open QPACK encoder stream: %w", err) + } + if err := openUniStream(ctx, conn, h3QPACKDecoderStream); err != nil { + return fmt.Errorf("open QPACK decoder stream: %w", err) + } + return nil +} + +func encodeRequestHeaders(authority string, tc requestCase) ([]byte, error) { + var block bytes.Buffer + + encoder := qpack.NewEncoder(&block) + fields := []qpack.HeaderField{ + {Name: ":method", Value: tc.method}, + {Name: ":scheme", Value: "https"}, + {Name: ":authority", Value: authority}, + {Name: ":path", Value: tc.path}, + {Name: "user-agent", Value: "ats-qmux-go-autest"}, + {Name: "x-qmux-client", Value: "qmux-go"}, + {Name: "x-qmux-test-case", Value: tc.name}, + {Name: "uuid", Value: tc.name}, + } + if tc.requestSize > 0 { + fields = append( + fields, + qpack.HeaderField{Name: "content-type", Value: "application/octet-stream"}, + qpack.HeaderField{Name: "content-length", Value: strconv.Itoa(tc.requestSize)}, + ) + } + for _, field := range fields { + if err := encoder.WriteField(field); err != nil { + return nil, err + } + } + return block.Bytes(), nil +} + +func decodeResponseHeaders(block []byte) (string, string, string, error) { + var status string + var marker string + var contentLength string + + decode := qpack.NewDecoder().Decode(block) + for { + field, err := decode() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return "", "", "", err + } + switch field.Name { + case ":status": + status = field.Value + case "x-qmux-response": + marker = field.Value + case "content-length": + contentLength = field.Value + } + } + return status, marker, contentLength, nil +} + +func readResponse(stream *qmux.Stream) (string, string, string, []byte, error) { + reader := quicvarint.NewReader(stream) + var status string + var marker string + var contentLength string + var body bytes.Buffer + + for { + frameType, err := quicvarint.Read(reader) + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return "", "", "", nil, err + } + length, err := quicvarint.Read(reader) + if err != nil { + return "", "", "", nil, err + } + payload := make([]byte, length) + if _, err := io.ReadFull(reader, payload); err != nil { Review Comment: `length` is a uint64, so `make([]byte, length)` won’t compile (slice lengths are `int`). Convert with an overflow guard before allocating. ########## tests/gold_tests/qmux/go_qmux_client/qmux_compat.go: ########## @@ -0,0 +1,223 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "bytes" + "fmt" + "io" + "net" + "sync" + + "github.com/quic-go/quic-go/quicvarint" +) + +const qmuxTransportParametersFrameType = 0x3f5153300d0a0d0a + +const ( + qmuxStreamFrameType = 0x08 + qmuxStreamFrameTypeMask = 0xf8 + qmuxStreamFrameOffsetBit = 0x04 + qmuxStreamFrameLengthBit = 0x02 +) + +// qmuxCompatConn adapts qmux-go v0.2.0's initial transport-parameter frame to +// draft-ietf-quic-qmux-01. The release omits the transport-parameter frame's +// payload length and cannot parse STREAM frames without a LEN field, so the +// adapter normalizes both differences before qmux-go sees them. +type qmuxCompatConn struct { + net.Conn + readMutex sync.Mutex + readBuffer bytes.Buffer + readReady bool + writeMutex sync.Mutex + writeDone bool +} + +func newQMuxCompatConn(conn net.Conn) net.Conn { + return &qmuxCompatConn{Conn: conn} +} + +func (conn *qmuxCompatConn) Read(data []byte) (int, error) { + conn.readMutex.Lock() + defer conn.readMutex.Unlock() + + if conn.readBuffer.Len() == 0 { + var adapted []byte + var err error + if conn.readReady { + adapted, err = conn.readRecord() + } else { + adapted, err = conn.readInitialRecord() + conn.readReady = true + } + if err != nil { + return 0, err + } + conn.readBuffer.Write(adapted) + } + return conn.readBuffer.Read(data) +} + +func (conn *qmuxCompatConn) readRecord() ([]byte, error) { + reader := quicvarint.NewReader(conn.Conn) + recordLength, err := quicvarint.Read(reader) + if err != nil { + return nil, err + } + payload := make([]byte, recordLength) + if _, err := io.ReadFull(reader, payload); err != nil { Review Comment: `recordLength` is a uint64, so `make([]byte, recordLength)` (and the following ReadFull) won’t compile because slice lengths are `int`. Convert to `int` with a guard to avoid overflow on 32-bit / extremely large records. ########## tests/gold_tests/qmux/go_qmux_client/qmux_compat.go: ########## @@ -0,0 +1,223 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "bytes" + "fmt" + "io" + "net" + "sync" + + "github.com/quic-go/quic-go/quicvarint" +) + +const qmuxTransportParametersFrameType = 0x3f5153300d0a0d0a + +const ( + qmuxStreamFrameType = 0x08 + qmuxStreamFrameTypeMask = 0xf8 + qmuxStreamFrameOffsetBit = 0x04 + qmuxStreamFrameLengthBit = 0x02 +) + +// qmuxCompatConn adapts qmux-go v0.2.0's initial transport-parameter frame to +// draft-ietf-quic-qmux-01. The release omits the transport-parameter frame's +// payload length and cannot parse STREAM frames without a LEN field, so the +// adapter normalizes both differences before qmux-go sees them. +type qmuxCompatConn struct { + net.Conn + readMutex sync.Mutex + readBuffer bytes.Buffer + readReady bool + writeMutex sync.Mutex + writeDone bool +} + +func newQMuxCompatConn(conn net.Conn) net.Conn { + return &qmuxCompatConn{Conn: conn} +} + +func (conn *qmuxCompatConn) Read(data []byte) (int, error) { + conn.readMutex.Lock() + defer conn.readMutex.Unlock() + + if conn.readBuffer.Len() == 0 { + var adapted []byte + var err error + if conn.readReady { + adapted, err = conn.readRecord() + } else { + adapted, err = conn.readInitialRecord() + conn.readReady = true + } + if err != nil { + return 0, err + } + conn.readBuffer.Write(adapted) + } + return conn.readBuffer.Read(data) +} + +func (conn *qmuxCompatConn) readRecord() ([]byte, error) { + reader := quicvarint.NewReader(conn.Conn) + recordLength, err := quicvarint.Read(reader) + if err != nil { + return nil, err + } + payload := make([]byte, recordLength) + if _, err := io.ReadFull(reader, payload); err != nil { + return nil, err + } + return adaptStreamFrameRecord(payload) +} + +func adaptStreamFrameRecord(payload []byte) ([]byte, error) { + frameType, frameTypeBytes, err := quicvarint.Parse(payload) + if err != nil { + return nil, err + } + if frameType&qmuxStreamFrameTypeMask != qmuxStreamFrameType || frameType&qmuxStreamFrameLengthBit != 0 { + return appendRecord(nil, payload), nil + } + + headerEnd := frameTypeBytes + _, streamIDBytes, err := quicvarint.Parse(payload[headerEnd:]) + if err != nil { + return nil, err + } + headerEnd += streamIDBytes + if frameType&qmuxStreamFrameOffsetBit != 0 { + _, offsetBytes, err := quicvarint.Parse(payload[headerEnd:]) + if err != nil { + return nil, err + } + headerEnd += offsetBytes + } + + adaptedPayload := quicvarint.Append(nil, frameType|qmuxStreamFrameLengthBit) + adaptedPayload = append(adaptedPayload, payload[frameTypeBytes:headerEnd]...) + adaptedPayload = quicvarint.Append(adaptedPayload, uint64(len(payload)-headerEnd)) + adaptedPayload = append(adaptedPayload, payload[headerEnd:]...) + return appendRecord(nil, adaptedPayload), nil +} + +func (conn *qmuxCompatConn) readInitialRecord() ([]byte, error) { + reader := quicvarint.NewReader(conn.Conn) + recordLength, err := quicvarint.Read(reader) + if err != nil { + return nil, err + } + payload := make([]byte, recordLength) + if _, err := io.ReadFull(reader, payload); err != nil { + return nil, err + } + + payloadReader := bytes.NewReader(payload) + frameType, err := quicvarint.Read(quicvarint.NewReader(payloadReader)) + if err != nil { + return nil, err + } + if frameType != qmuxTransportParametersFrameType { + return nil, fmt.Errorf("expected initial QX_TRANSPORT_PARAMETERS frame, got %#x", frameType) + } + parameterLength, err := quicvarint.Read(quicvarint.NewReader(payloadReader)) + if err != nil { + return nil, err + } + if parameterLength > uint64(payloadReader.Len()) { + return nil, fmt.Errorf("QMux transport parameters length %d exceeds record payload", parameterLength) + } + + parameterBytes := make([]byte, parameterLength) Review Comment: `parameterLength` is a uint64, so `make([]byte, parameterLength)` won’t compile. You already bound-check `parameterLength` against `payloadReader.Len()` (an `int`), so it’s safe to cast here. ########## tests/gold_tests/qmux/go_qmux_client/qmux_compat.go: ########## @@ -0,0 +1,223 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "bytes" + "fmt" + "io" + "net" + "sync" + + "github.com/quic-go/quic-go/quicvarint" +) + +const qmuxTransportParametersFrameType = 0x3f5153300d0a0d0a + +const ( + qmuxStreamFrameType = 0x08 + qmuxStreamFrameTypeMask = 0xf8 + qmuxStreamFrameOffsetBit = 0x04 + qmuxStreamFrameLengthBit = 0x02 +) + +// qmuxCompatConn adapts qmux-go v0.2.0's initial transport-parameter frame to +// draft-ietf-quic-qmux-01. The release omits the transport-parameter frame's +// payload length and cannot parse STREAM frames without a LEN field, so the +// adapter normalizes both differences before qmux-go sees them. +type qmuxCompatConn struct { + net.Conn + readMutex sync.Mutex + readBuffer bytes.Buffer + readReady bool + writeMutex sync.Mutex + writeDone bool +} + +func newQMuxCompatConn(conn net.Conn) net.Conn { + return &qmuxCompatConn{Conn: conn} +} + +func (conn *qmuxCompatConn) Read(data []byte) (int, error) { + conn.readMutex.Lock() + defer conn.readMutex.Unlock() + + if conn.readBuffer.Len() == 0 { + var adapted []byte + var err error + if conn.readReady { + adapted, err = conn.readRecord() + } else { + adapted, err = conn.readInitialRecord() + conn.readReady = true + } + if err != nil { + return 0, err + } + conn.readBuffer.Write(adapted) + } + return conn.readBuffer.Read(data) +} + +func (conn *qmuxCompatConn) readRecord() ([]byte, error) { + reader := quicvarint.NewReader(conn.Conn) + recordLength, err := quicvarint.Read(reader) + if err != nil { + return nil, err + } + payload := make([]byte, recordLength) + if _, err := io.ReadFull(reader, payload); err != nil { + return nil, err + } + return adaptStreamFrameRecord(payload) +} + +func adaptStreamFrameRecord(payload []byte) ([]byte, error) { + frameType, frameTypeBytes, err := quicvarint.Parse(payload) + if err != nil { + return nil, err + } + if frameType&qmuxStreamFrameTypeMask != qmuxStreamFrameType || frameType&qmuxStreamFrameLengthBit != 0 { + return appendRecord(nil, payload), nil + } + + headerEnd := frameTypeBytes + _, streamIDBytes, err := quicvarint.Parse(payload[headerEnd:]) + if err != nil { + return nil, err + } + headerEnd += streamIDBytes + if frameType&qmuxStreamFrameOffsetBit != 0 { + _, offsetBytes, err := quicvarint.Parse(payload[headerEnd:]) + if err != nil { + return nil, err + } + headerEnd += offsetBytes + } + + adaptedPayload := quicvarint.Append(nil, frameType|qmuxStreamFrameLengthBit) + adaptedPayload = append(adaptedPayload, payload[frameTypeBytes:headerEnd]...) + adaptedPayload = quicvarint.Append(adaptedPayload, uint64(len(payload)-headerEnd)) + adaptedPayload = append(adaptedPayload, payload[headerEnd:]...) + return appendRecord(nil, adaptedPayload), nil +} + +func (conn *qmuxCompatConn) readInitialRecord() ([]byte, error) { + reader := quicvarint.NewReader(conn.Conn) + recordLength, err := quicvarint.Read(reader) + if err != nil { + return nil, err + } + payload := make([]byte, recordLength) + if _, err := io.ReadFull(reader, payload); err != nil { Review Comment: Same issue as above in `readInitialRecord`: `recordLength` is uint64, but slice sizes must be `int`. This currently won’t compile. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
