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

implement the datagram draft #2162

Merged
merged 15 commits into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions internal/flowcontrol/stream_flow_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ var _ = Describe("Stream Flow controller", func() {

Context("Constructor", func() {
rttStats := &utils.RTTStats{}
receiveWindow := protocol.ByteCount(2000)
maxReceiveWindow := protocol.ByteCount(3000)
sendWindow := protocol.ByteCount(4000)
const receiveWindow protocol.ByteCount = 2000
const maxReceiveWindow protocol.ByteCount = 3000
const sendWindow protocol.ByteCount = 4000

It("sets the send and receive windows", func() {
cc := NewConnectionFlowController(0, 0, nil, nil, utils.DefaultLogger)
Expand All @@ -50,7 +50,7 @@ var _ = Describe("Stream Flow controller", func() {
queued = true
}

cc := NewConnectionFlowController(0, 0, nil, nil, utils.DefaultLogger)
cc := NewConnectionFlowController(receiveWindow, maxReceiveWindow, func() {}, nil, utils.DefaultLogger)
fc := NewStreamFlowController(5, cc, receiveWindow, maxReceiveWindow, sendWindow, queueWindowUpdate, rttStats, utils.DefaultLogger).(*streamFlowController)
fc.AddBytesRead(receiveWindow)
Expect(queued).To(BeTrue())
Expand Down
4 changes: 4 additions & 0 deletions internal/logutils/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ func ConvertFrame(frame wire.Frame) logging.Frame {
Length: f.DataLen(),
Fin: f.Fin,
}
case *wire.DatagramFrame:
return &logging.DatagramFrame{
Length: logging.ByteCount(len(f.Data)),
}
default:
return logging.Frame(frame)
}
Expand Down
7 changes: 7 additions & 0 deletions internal/logutils/frame_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ var _ = Describe("CRYPTO frame", func() {
Expect(sf.Fin).To(BeTrue())
})

It("converts DATAGRAM frames", func() {
f := ConvertFrame(&wire.DatagramFrame{Data: []byte("foobar")})
Expect(f).To(BeAssignableToTypeOf(&logging.DatagramFrame{}))
df := f.(*logging.DatagramFrame)
Expect(df.Length).To(Equal(logging.ByteCount(6)))
})

It("converts other frames", func() {
f := ConvertFrame(&wire.MaxDataFrame{MaximumData: 1234})
Expect(f).To(BeAssignableToTypeOf(&logging.MaxDataFrame{}))
Expand Down
5 changes: 4 additions & 1 deletion internal/protocol/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,14 @@ const (
)

// A ByteCount in QUIC
type ByteCount uint64
type ByteCount int64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems scary to me :S Are we sure this doesn't break any assumptions around non-negative numbers sprinkled throughout the codebase?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did uncover a uint underflow, fortunately it was just in test code (in the flow controller). I'm not sure why this is scary, as QUIC's varint encoding makes sure that all numbers that can practically be used are limited to MaxUint64 / 4.


// MaxByteCount is the maximum value of a ByteCount
const MaxByteCount = ByteCount(1<<62 - 1)

// InvalidByteCount is an invalid byte count
const InvalidByteCount ByteCount = -1

// An ApplicationErrorCode is an application-defined error code.
type ApplicationErrorCode uint64

Expand Down
85 changes: 85 additions & 0 deletions internal/wire/datagram_frame.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package wire

import (
"bytes"
"io"

"github.com/lucas-clemente/quic-go/internal/protocol"
"github.com/lucas-clemente/quic-go/internal/utils"
)

// A DatagramFrame is a DATAGRAM frame
type DatagramFrame struct {
DataLenPresent bool
Data []byte
}

func parseDatagramFrame(r *bytes.Reader, _ protocol.VersionNumber) (*DatagramFrame, error) {
typeByte, err := r.ReadByte()
if err != nil {
return nil, err
}

f := &DatagramFrame{}
f.DataLenPresent = typeByte&0x1 > 0

var length uint64
if f.DataLenPresent {
var err error
len, err := utils.ReadVarInt(r)
if err != nil {
return nil, err
}
if len > uint64(r.Len()) {
return nil, io.EOF
}
length = len
} else {
length = uint64(r.Len())
}
f.Data = make([]byte, length)
if _, err := io.ReadFull(r, f.Data); err != nil {
return nil, err
}
return f, nil
}

func (f *DatagramFrame) Write(b *bytes.Buffer, _ protocol.VersionNumber) error {
typeByte := uint8(0x30)
if f.DataLenPresent {
typeByte ^= 0x1
}
b.WriteByte(typeByte)
if f.DataLenPresent {
utils.WriteVarInt(b, uint64(len(f.Data)))
}
b.Write(f.Data)
return nil
}

// MaxDataLen returns the maximum data length
func (f *DatagramFrame) MaxDataLen(maxSize protocol.ByteCount, version protocol.VersionNumber) protocol.ByteCount {
headerLen := protocol.ByteCount(1)
if f.DataLenPresent {
// pretend that the data size will be 1 bytes
// if it turns out that varint encoding the length will consume 2 bytes, we need to adjust the data length afterwards
headerLen++
}
if headerLen > maxSize {
return 0
}
maxDataLen := maxSize - headerLen
if f.DataLenPresent && utils.VarIntLen(uint64(maxDataLen)) != 1 {
maxDataLen--
}
return maxDataLen
}

// Length of a written frame
func (f *DatagramFrame) Length(_ protocol.VersionNumber) protocol.ByteCount {
length := 1 + protocol.ByteCount(len(f.Data))
if f.DataLenPresent {
length += utils.VarIntLen(uint64(len(f.Data)))
}
return length
}
153 changes: 153 additions & 0 deletions internal/wire/datagram_frame_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
package wire

import (
"bytes"
"io"

"github.com/lucas-clemente/quic-go/internal/protocol"
"github.com/lucas-clemente/quic-go/internal/utils"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

var _ = Describe("STREAM frame", func() {
Context("when parsing", func() {
It("parses a frame containing a length", func() {
data := []byte{0x30 ^ 0x1}
data = append(data, encodeVarInt(0x6)...) // length
data = append(data, []byte("foobar")...)
r := bytes.NewReader(data)
frame, err := parseDatagramFrame(r, versionIETFFrames)
Expect(err).ToNot(HaveOccurred())
Expect(frame.Data).To(Equal([]byte("foobar")))
Expect(frame.DataLenPresent).To(BeTrue())
Expect(r.Len()).To(BeZero())
})

It("parses a frame without length", func() {
data := []byte{0x30}
data = append(data, []byte("Lorem ipsum dolor sit amet")...)
r := bytes.NewReader(data)
frame, err := parseDatagramFrame(r, versionIETFFrames)
Expect(err).ToNot(HaveOccurred())
Expect(frame.Data).To(Equal([]byte("Lorem ipsum dolor sit amet")))
Expect(frame.DataLenPresent).To(BeFalse())
Expect(r.Len()).To(BeZero())
})

It("errors when the length is longer than the rest of the frame", func() {
data := []byte{0x30 ^ 0x1}
data = append(data, encodeVarInt(0x6)...) // length
data = append(data, []byte("fooba")...)
r := bytes.NewReader(data)
_, err := parseDatagramFrame(r, versionIETFFrames)
Expect(err).To(MatchError(io.EOF))
})

It("errors on EOFs", func() {
data := []byte{0x30 ^ 0x1}
data = append(data, encodeVarInt(6)...) // length
data = append(data, []byte("foobar")...)
_, err := parseDatagramFrame(bytes.NewReader(data), versionIETFFrames)
Expect(err).NotTo(HaveOccurred())
for i := range data {
_, err := parseDatagramFrame(bytes.NewReader(data[0:i]), versionIETFFrames)
Expect(err).To(MatchError(io.EOF))
}
})
})

Context("when writing", func() {
It("writes a frame with length", func() {
f := &DatagramFrame{
DataLenPresent: true,
Data: []byte("foobar"),
}
buf := &bytes.Buffer{}
Expect(f.Write(buf, versionIETFFrames)).To(Succeed())
expected := []byte{0x30 ^ 0x1}
expected = append(expected, encodeVarInt(0x6)...)
expected = append(expected, []byte("foobar")...)
Expect(buf.Bytes()).To(Equal(expected))
})

It("writes a frame without length", func() {
f := &DatagramFrame{Data: []byte("Lorem ipsum")}
buf := &bytes.Buffer{}
Expect(f.Write(buf, versionIETFFrames)).To(Succeed())
expected := []byte{0x30}
expected = append(expected, []byte("Lorem ipsum")...)
Expect(buf.Bytes()).To(Equal(expected))
})
})

Context("length", func() {
It("has the right length for a frame with length", func() {
f := &DatagramFrame{
DataLenPresent: true,
Data: []byte("foobar"),
}
Expect(f.Length(versionIETFFrames)).To(Equal(1 + utils.VarIntLen(6) + 6))
})

It("has the right length for a frame without length", func() {
f := &DatagramFrame{Data: []byte("foobar")}
Expect(f.Length(versionIETFFrames)).To(Equal(protocol.ByteCount(1 + 6)))
})
})

Context("max data length", func() {
const maxSize = 3000

It("returns a data length such that the resulting frame has the right size, if data length is not present", func() {
data := make([]byte, maxSize)
f := &DatagramFrame{}
b := &bytes.Buffer{}
for i := 1; i < 3000; i++ {
b.Reset()
f.Data = nil
maxDataLen := f.MaxDataLen(protocol.ByteCount(i), versionIETFFrames)
if maxDataLen == 0 { // 0 means that no valid STREAM frame can be written
// check that writing a minimal size STREAM frame (i.e. with 1 byte data) is actually larger than the desired size
f.Data = []byte{0}
Expect(f.Write(b, versionIETFFrames)).To(Succeed())
Expect(b.Len()).To(BeNumerically(">", i))
continue
}
f.Data = data[:int(maxDataLen)]
Expect(f.Write(b, versionIETFFrames)).To(Succeed())
Expect(b.Len()).To(Equal(i))
}
})

It("always returns a data length such that the resulting frame has the right size, if data length is present", func() {
data := make([]byte, maxSize)
f := &DatagramFrame{DataLenPresent: true}
b := &bytes.Buffer{}
var frameOneByteTooSmallCounter int
for i := 1; i < 3000; i++ {
b.Reset()
f.Data = nil
maxDataLen := f.MaxDataLen(protocol.ByteCount(i), versionIETFFrames)
if maxDataLen == 0 { // 0 means that no valid STREAM frame can be written
// check that writing a minimal size STREAM frame (i.e. with 1 byte data) is actually larger than the desired size
f.Data = []byte{0}
Expect(f.Write(b, versionIETFFrames)).To(Succeed())
Expect(b.Len()).To(BeNumerically(">", i))
continue
}
f.Data = data[:int(maxDataLen)]
Expect(f.Write(b, versionIETFFrames)).To(Succeed())
// There's *one* pathological case, where a data length of x can be encoded into 1 byte
// but a data lengths of x+1 needs 2 bytes
// In that case, it's impossible to create a STREAM frame of the desired size
if b.Len() == i-1 {
frameOneByteTooSmallCounter++
continue
}
Expect(b.Len()).To(Equal(i))
}
Expect(frameOneByteTooSmallCounter).To(Equal(1))
})
})
})
2 changes: 2 additions & 0 deletions internal/wire/frame_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ func (p *frameParser) parseFrame(r *bytes.Reader, typeByte byte, encLevel protoc
frame, err = parseConnectionCloseFrame(r, p.version)
case 0x1e:
frame, err = parseHandshakeDoneFrame(r, p.version)
case 0x30, 0x31:
frame, err = parseDatagramFrame(r, p.version)
default:
err = errors.New("unknown frame type")
}
Expand Down
1 change: 1 addition & 0 deletions internal/wire/frame_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ var _ = Describe("Frame parsing", func() {
&PathResponseFrame{},
&ConnectionCloseFrame{},
&HandshakeDoneFrame{},
&DatagramFrame{},
}

var framesSerialized [][]byte
Expand Down
5 changes: 5 additions & 0 deletions logging/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,8 @@ type StreamFrame struct {
Length ByteCount
Fin bool
}

// A DatagramFrame is a DATAGRAM frame.
type DatagramFrame struct {
Length ByteCount
}
7 changes: 7 additions & 0 deletions qlog/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ func (f frame) MarshalJSONObject(enc *gojay.Encoder) {
marshalConnectionCloseFrame(enc, frame)
case *logging.HandshakeDoneFrame:
marshalHandshakeDoneFrame(enc, frame)
case *logging.DatagramFrame:
marshalDatagramFrame(enc, frame)
default:
panic("unknown frame type")
}
Expand Down Expand Up @@ -218,3 +220,8 @@ func marshalConnectionCloseFrame(enc *gojay.Encoder, f *logging.ConnectionCloseF
func marshalHandshakeDoneFrame(enc *gojay.Encoder, _ *logging.HandshakeDoneFrame) {
enc.StringKey("frame_type", "handshake_done")
}

func marshalDatagramFrame(enc *gojay.Encoder, f *logging.DatagramFrame) {
enc.StringKey("frame_type", "datagram")
enc.Int64Key("length", int64(f.Length))
}
10 changes: 10 additions & 0 deletions qlog/frame_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,4 +364,14 @@ var _ = Describe("Frames", func() {
},
)
})

It("marshals DATAGRAM frames", func() {
check(
&logging.DatagramFrame{Length: 1337},
map[string]interface{}{
"frame_type": "datagram",
"length": 1337,
},
)
})
})