remove the error return value from wire.Frame.MinLength

No functional change expected.
The error was only non-nil if some required values for the STOP_WAITING
frame were not set. It should be sufficient to throw an error when
attempting to write an invalid STOP_WAITING frame.
This commit is contained in:
Marten Seemann 2017-12-12 10:48:17 +07:00
parent ded0eb4f6f
commit 4b4e487486
20 changed files with 51 additions and 83 deletions

View file

@ -139,7 +139,7 @@ func (f *AckFrame) Write(b *bytes.Buffer, version protocol.VersionNumber) error
}
// MinLength of a written frame
func (f *AckFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCount, error) {
func (f *AckFrame) MinLength(version protocol.VersionNumber) protocol.ByteCount {
if !version.UsesIETFFrameFormat() {
return f.minLengthLegacy(version)
}
@ -157,7 +157,7 @@ func (f *AckFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCount
length += utils.VarIntLen(uint64(f.LargestAcked - lowestInFirstRange))
if !f.HasMissingRanges() {
return length, nil
return length
}
var lowest protocol.PacketNumber
for i, ackRange := range f.AckRanges {
@ -169,7 +169,7 @@ func (f *AckFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCount
length += utils.VarIntLen(uint64(ackRange.Last - ackRange.First))
lowest = ackRange.First
}
return length, nil
return length
}
// HasMissingRanges returns if this frame reports any missing packets

View file

@ -308,7 +308,7 @@ func (f *AckFrame) writeLegacy(b *bytes.Buffer, _ protocol.VersionNumber) error
return nil
}
func (f *AckFrame) minLengthLegacy(_ protocol.VersionNumber) (protocol.ByteCount, error) {
func (f *AckFrame) minLengthLegacy(_ protocol.VersionNumber) protocol.ByteCount {
length := protocol.ByteCount(1 + 2 + 1) // 1 TypeByte, 2 ACK delay time, 1 Num Timestamp
length += protocol.ByteCount(protocol.GetPacketNumberLength(f.LargestAcked))
@ -320,7 +320,7 @@ func (f *AckFrame) minLengthLegacy(_ protocol.VersionNumber) (protocol.ByteCount
length += missingSequenceNumberDeltaLen
}
// we don't write
return length, nil
return length
}
// numWritableNackRanges calculates the number of ACK blocks that are about to be written

View file

@ -27,9 +27,9 @@ func (f *BlockedFrame) Write(b *bytes.Buffer, version protocol.VersionNumber) er
}
// MinLength of a written frame
func (f *BlockedFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCount, error) {
func (f *BlockedFrame) MinLength(version protocol.VersionNumber) protocol.ByteCount {
if !version.UsesIETFFrameFormat() { // writing this frame would result in a legacy BLOCKED being written, which is longer
return 1 + 4, nil
return 1 + 4
}
return 1, nil
return 1
}

View file

@ -68,11 +68,11 @@ func ParseConnectionCloseFrame(r *bytes.Reader, version protocol.VersionNumber)
}
// MinLength of a written frame
func (f *ConnectionCloseFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCount, error) {
func (f *ConnectionCloseFrame) MinLength(version protocol.VersionNumber) protocol.ByteCount {
if version.UsesIETFFrameFormat() {
return 1 + 2 + utils.VarIntLen(uint64(len(f.ReasonPhrase))) + protocol.ByteCount(len(f.ReasonPhrase)), nil
return 1 + 2 + utils.VarIntLen(uint64(len(f.ReasonPhrase))) + protocol.ByteCount(len(f.ReasonPhrase))
}
return 1 + 4 + 2 + protocol.ByteCount(len(f.ReasonPhrase)), nil
return 1 + 4 + 2 + protocol.ByteCount(len(f.ReasonPhrase))
}
// Write writes an CONNECTION_CLOSE frame.

View file

@ -9,5 +9,5 @@ import (
// A Frame in QUIC
type Frame interface {
Write(b *bytes.Buffer, version protocol.VersionNumber) error
MinLength(version protocol.VersionNumber) (protocol.ByteCount, error)
MinLength(version protocol.VersionNumber) protocol.ByteCount
}

View file

@ -63,6 +63,6 @@ func (f *GoawayFrame) Write(b *bytes.Buffer, _ protocol.VersionNumber) error {
}
// MinLength of a written frame
func (f *GoawayFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCount, error) {
return protocol.ByteCount(1 + 4 + 4 + 2 + len(f.ReasonPhrase)), nil
func (f *GoawayFrame) MinLength(version protocol.VersionNumber) protocol.ByteCount {
return protocol.ByteCount(1 + 4 + 4 + 2 + len(f.ReasonPhrase))
}

View file

@ -43,9 +43,9 @@ func (f *MaxDataFrame) Write(b *bytes.Buffer, version protocol.VersionNumber) er
}
// MinLength of a written frame
func (f *MaxDataFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCount, error) {
func (f *MaxDataFrame) MinLength(version protocol.VersionNumber) protocol.ByteCount {
if !version.UsesIETFFrameFormat() { // writing this frame would result in a gQUIC WINDOW_UPDATE being written, which is longer
return 1 + 4 + 8, nil
return 1 + 4 + 8
}
return 1 + utils.VarIntLen(uint64(f.ByteOffset)), nil
return 1 + utils.VarIntLen(uint64(f.ByteOffset))
}

View file

@ -51,10 +51,10 @@ func (f *MaxStreamDataFrame) Write(b *bytes.Buffer, version protocol.VersionNumb
}
// MinLength of a written frame
func (f *MaxStreamDataFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCount, error) {
func (f *MaxStreamDataFrame) MinLength(version protocol.VersionNumber) protocol.ByteCount {
// writing this frame would result in a gQUIC WINDOW_UPDATE being written, which has a different length
if !version.UsesIETFFrameFormat() {
return 1 + 4 + 8, nil
return 1 + 4 + 8
}
return 1 + utils.VarIntLen(uint64(f.StreamID)) + utils.VarIntLen(uint64(f.ByteOffset)), nil
return 1 + utils.VarIntLen(uint64(f.StreamID)) + utils.VarIntLen(uint64(f.ByteOffset))
}

View file

@ -28,6 +28,6 @@ func (f *PingFrame) Write(b *bytes.Buffer, version protocol.VersionNumber) error
}
// MinLength of a written frame
func (f *PingFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCount, error) {
return 1, nil
func (f *PingFrame) MinLength(version protocol.VersionNumber) protocol.ByteCount {
return 1
}

View file

@ -80,9 +80,9 @@ func (f *RstStreamFrame) Write(b *bytes.Buffer, version protocol.VersionNumber)
}
// MinLength of a written frame
func (f *RstStreamFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCount, error) {
func (f *RstStreamFrame) MinLength(version protocol.VersionNumber) protocol.ByteCount {
if version.UsesIETFFrameFormat() {
return 1 + utils.VarIntLen(uint64(f.StreamID)) + 2 + utils.VarIntLen(uint64(f.ByteOffset)), nil
return 1 + utils.VarIntLen(uint64(f.StreamID)) + 2 + utils.VarIntLen(uint64(f.ByteOffset))
}
return 1 + 4 + 8 + 4, nil
return 1 + 4 + 8 + 4
}

View file

@ -49,14 +49,8 @@ func (f *StopWaitingFrame) Write(b *bytes.Buffer, _ protocol.VersionNumber) erro
}
// MinLength of a written frame
func (f *StopWaitingFrame) MinLength(_ protocol.VersionNumber) (protocol.ByteCount, error) {
minLength := protocol.ByteCount(1) // typeByte
if f.PacketNumberLen == protocol.PacketNumberLenInvalid {
return 0, errPacketNumberLenNotSet
}
minLength += protocol.ByteCount(f.PacketNumberLen)
return minLength, nil
func (f *StopWaitingFrame) MinLength(_ protocol.VersionNumber) protocol.ByteCount {
return 1 + protocol.ByteCount(f.PacketNumberLen)
}
// ParseStopWaitingFrame parses a StopWaiting frame

View file

@ -176,14 +176,6 @@ var _ = Describe("StopWaitingFrame", func() {
Expect(frame.MinLength(protocol.VersionWhatever)).To(Equal(protocol.ByteCount(length + 1)))
}
})
It("errors when packetNumberLen is not set", func() {
frame := &StopWaitingFrame{
LeastUnacked: 10,
}
_, err := frame.MinLength(0)
Expect(err).To(MatchError(errPacketNumberLenNotSet))
})
})
Context("self consistency", func() {

View file

@ -35,9 +35,9 @@ func (f *StreamBlockedFrame) Write(b *bytes.Buffer, version protocol.VersionNumb
}
// MinLength of a written frame
func (f *StreamBlockedFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCount, error) {
func (f *StreamBlockedFrame) MinLength(version protocol.VersionNumber) protocol.ByteCount {
if !version.UsesIETFFrameFormat() {
return 1 + 4, nil
return 1 + 4
}
return 1 + utils.VarIntLen(uint64(f.StreamID)), nil
return 1 + utils.VarIntLen(uint64(f.StreamID))
}

View file

@ -117,7 +117,7 @@ func (f *StreamFrame) Write(b *bytes.Buffer, version protocol.VersionNumber) err
// MinLength returns the length of the header of a StreamFrame
// the total length of the frame is frame.MinLength() + frame.DataLen()
func (f *StreamFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCount, error) {
func (f *StreamFrame) MinLength(version protocol.VersionNumber) protocol.ByteCount {
if !version.UsesIETFFrameFormat() {
return f.minLengthLegacy(version)
}
@ -128,5 +128,5 @@ func (f *StreamFrame) MinLength(version protocol.VersionNumber) (protocol.ByteCo
if f.DataLenPresent {
length += utils.VarIntLen(uint64(f.DataLen()))
}
return length, nil
return length
}

View file

@ -183,12 +183,12 @@ func (f *StreamFrame) getOffsetLength() protocol.ByteCount {
return 8
}
func (f *StreamFrame) minLengthLegacy(_ protocol.VersionNumber) (protocol.ByteCount, error) {
func (f *StreamFrame) minLengthLegacy(_ protocol.VersionNumber) protocol.ByteCount {
length := protocol.ByteCount(1) + protocol.ByteCount(f.calculateStreamIDLength()) + f.getOffsetLength()
if f.DataLenPresent {
length += 2
}
return length, nil
return length
}
// DataLen gives the length of data in bytes

View file

@ -210,7 +210,7 @@ var _ = Describe("STREAM frame (for gQUIC)", func() {
}
err := f.Write(b, versionBigEndian)
Expect(err).ToNot(HaveOccurred())
minLength, _ := f.MinLength(0)
minLength := f.MinLength(0)
Expect(b.Bytes()[0] & 0x20).To(Equal(uint8(0x20)))
Expect(b.Bytes()[minLength-2 : minLength]).To(Equal([]byte{0x13, 0x37}))
})
@ -229,9 +229,9 @@ var _ = Describe("STREAM frame (for gQUIC)", func() {
Expect(err).ToNot(HaveOccurred())
Expect(b.Bytes()[0] & 0x20).To(Equal(uint8(0)))
Expect(b.Bytes()[1 : b.Len()-dataLen]).ToNot(ContainSubstring(string([]byte{0x37, 0x13})))
minLength, _ := f.MinLength(versionBigEndian)
minLength := f.MinLength(versionBigEndian)
f.DataLenPresent = true
minLengthWithoutDataLen, _ := f.MinLength(versionBigEndian)
minLengthWithoutDataLen := f.MinLength(versionBigEndian)
Expect(minLength).To(Equal(minLengthWithoutDataLen - 2))
})
@ -242,7 +242,7 @@ var _ = Describe("STREAM frame (for gQUIC)", func() {
DataLenPresent: false,
Offset: 0xdeadbeef,
}
minLengthWithoutDataLen, _ := f.MinLength(versionBigEndian)
minLengthWithoutDataLen := f.MinLength(versionBigEndian)
f.DataLenPresent = true
Expect(f.MinLength(versionBigEndian)).To(Equal(minLengthWithoutDataLen + 2))
})

View file

@ -199,27 +199,17 @@ func (p *packetPacker) composeNextPacket(
// STOP_WAITING and ACK will always fit
if p.stopWaiting != nil {
payloadFrames = append(payloadFrames, p.stopWaiting)
l, err := p.stopWaiting.MinLength(p.version)
if err != nil {
return nil, err
}
payloadLength += l
payloadLength += p.stopWaiting.MinLength(p.version)
}
if p.ackFrame != nil {
payloadFrames = append(payloadFrames, p.ackFrame)
l, err := p.ackFrame.MinLength(p.version)
if err != nil {
return nil, err
}
l := p.ackFrame.MinLength(p.version)
payloadLength += l
}
for len(p.controlFrames) > 0 {
frame := p.controlFrames[len(p.controlFrames)-1]
minLength, err := frame.MinLength(p.version)
if err != nil {
return nil, err
}
minLength := frame.MinLength(p.version)
if payloadLength+minLength > maxFrameSize {
break
}

View file

@ -329,8 +329,7 @@ var _ = Describe("Packet packer", func() {
It("packs a lot of control frames into 2 packets if they don't fit into one", func() {
blockedFrame := &wire.BlockedFrame{}
minLength, _ := blockedFrame.MinLength(packer.version)
maxFramesPerPacket := int(maxFrameSize) / int(minLength)
maxFramesPerPacket := int(maxFrameSize) / int(blockedFrame.MinLength(packer.version))
var controlFrames []wire.Frame
for i := 0; i < maxFramesPerPacket+10; i++ {
controlFrames = append(controlFrames, blockedFrame)
@ -369,8 +368,7 @@ var _ = Describe("Packet packer", func() {
StreamID: 5,
DataLenPresent: false,
}
minLength, _ := f.MinLength(packer.version)
maxStreamFrameDataLen := maxFrameSize - minLength
maxStreamFrameDataLen := maxFrameSize - f.MinLength(packer.version)
f.Data = bytes.Repeat([]byte{'f'}, int(maxStreamFrameDataLen))
streamFramer.AddFrameForRetransmission(f)
payloadFrames, err := packer.composeNextPacket(maxFrameSize, true)
@ -390,10 +388,9 @@ var _ = Describe("Packet packer", func() {
StreamID: 5,
DataLenPresent: true,
}
minLength, _ := f.MinLength(packer.version)
// for IETF draft style STREAM frames, we don't know the size of the DataLen, because it is a variable length integer
// in the general case, we therefore use a STREAM frame that is 1 byte smaller than the maximum size
maxStreamFrameDataLen := maxFrameSize - minLength - 1
maxStreamFrameDataLen := maxFrameSize - f.MinLength(packer.version) - 1
f.Data = bytes.Repeat([]byte{'f'}, int(maxStreamFrameDataLen))
streamFramer.AddFrameForRetransmission(f)
payloadFrames, err := packer.composeNextPacket(maxFrameSize, true)
@ -467,8 +464,7 @@ var _ = Describe("Packet packer", func() {
StreamID: 7,
Offset: 1,
}
minLength, _ := f.MinLength(packer.version)
maxStreamFrameDataLen := maxFrameSize - minLength
maxStreamFrameDataLen := maxFrameSize - f.MinLength(packer.version)
f.Data = bytes.Repeat([]byte{'f'}, int(maxStreamFrameDataLen)+200)
streamFramer.AddFrameForRetransmission(f)
payloadFrames, err := packer.composeNextPacket(maxFrameSize, true)
@ -526,8 +522,7 @@ var _ = Describe("Packet packer", func() {
StreamID: 5,
Offset: 1,
}
minLength, _ := f.MinLength(packer.version)
f.Data = bytes.Repeat([]byte{'f'}, int(maxFrameSize-minLength+1)) // + 1 since MinceLength is 1 bigger than the actual StreamFrame header
f.Data = bytes.Repeat([]byte{'f'}, int(maxFrameSize-f.MinLength(packer.version)+1)) // + 1 since MinceLength is 1 bigger than the actual StreamFrame header
streamFramer.AddFrameForRetransmission(f)
p, err := packer.PackPacket()
Expect(err).ToNot(HaveOccurred())
@ -540,8 +535,7 @@ var _ = Describe("Packet packer", func() {
StreamID: 5,
Offset: 1,
}
minLength, _ := f.MinLength(packer.version)
f.Data = bytes.Repeat([]byte{'f'}, int(maxFrameSize-minLength+2)) // + 2 since MinceLength is 1 bigger than the actual StreamFrame header
f.Data = bytes.Repeat([]byte{'f'}, int(maxFrameSize-f.MinLength(packer.version)+2)) // + 2 since MinceLength is 1 bigger than the actual StreamFrame header
streamFramer.AddFrameForRetransmission(f)
payloadFrames, err := packer.composeNextPacket(maxFrameSize, true)

View file

@ -66,8 +66,7 @@ func (f *streamFramer) PopCryptoStreamFrame(maxLen protocol.ByteCount) *wire.Str
StreamID: f.cryptoStream.StreamID(),
Offset: f.cryptoStream.GetWriteOffset(),
}
frameHeaderBytes, _ := frame.MinLength(f.version) // can never error
frame.Data, frame.FinBit = f.cryptoStream.GetDataForWriting(maxLen - frameHeaderBytes)
frame.Data, frame.FinBit = f.cryptoStream.GetDataForWriting(maxLen - frame.MinLength(f.version))
return frame
}
@ -76,11 +75,10 @@ func (f *streamFramer) maybePopFramesForRetransmission(maxLen protocol.ByteCount
frame := f.retransmissionQueue[0]
frame.DataLenPresent = true
frameHeaderLen, _ := frame.MinLength(f.version) // can never error
frameHeaderLen := frame.MinLength(f.version)
if currentLen+frameHeaderLen >= maxLen {
break
}
currentLen += frameHeaderLen
splitFrame := maybeSplitOffFrame(frame, maxLen-currentLen)
@ -109,7 +107,7 @@ func (f *streamFramer) maybePopNormalFrames(maxBytes protocol.ByteCount) (res []
frame.StreamID = s.StreamID()
frame.Offset = s.GetWriteOffset()
// not perfect, but thread-safe since writeOffset is only written when getting data
frameHeaderBytes, _ := frame.MinLength(f.version) // can never error
frameHeaderBytes := frame.MinLength(f.version)
if currentLen+frameHeaderBytes > maxBytes {
return false, nil // theoretically, we could find another stream that fits, but this is quite unlikely, so we stop here
}

View file

@ -218,7 +218,7 @@ var _ = Describe("Stream Framer", func() {
origlen := retransmittedFrame2.DataLen()
fs := framer.PopStreamFrames(6)
Expect(fs).To(HaveLen(1))
minLength, _ := fs[0].MinLength(framer.version)
minLength := fs[0].MinLength(framer.version)
Expect(minLength + fs[0].DataLen()).To(Equal(protocol.ByteCount(6)))
Expect(framer.retransmissionQueue[0].Data).To(HaveLen(int(origlen - fs[0].DataLen())))
Expect(framer.retransmissionQueue[0].Offset).To(Equal(fs[0].DataLen()))