Repository: quic-go/quic-go
Pull request #4501 (2024-05-08 03:49:29)

wire: write configured value of max_udp_payload_size transport parameter
There's no reason the wire package should use the constant: It should be able to marshal any value configured.
Instead, we set the `MaxUDPPayloadSize` when initialising the connection.

No functional change expected.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/4501?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 85.12%. Comparing base [(`66f968b`)](https://app.codecov.io/gh/quic-go/quic-go/commit/66f968b9ff1d459ee7980d69dc6aa477e1980f1e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`73ff5c6`)](https://app.codecov.io/gh/quic-go/quic-go/pull/4501?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4501      +/-   ##
==========================================
- Coverage   85.15%   85.12%   -0.03%     
==========================================
  Files         154      151       -3     
  Lines       14768    14492     -276     
==========================================
- Hits        12575    12336     -239     
+ Misses       1688     1669      -19     
+ Partials      505      487      -18     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/4501?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).


Patch:
diff --git a/connection.go b/connection.go
index 0b009fbd..1abe00a0 100644
--- a/connection.go
+++ b/connection.go
@@ -299,6 +299,7 @@ var newConnection = func(
 		MaxUniStreamNum:                 protocol.StreamNum(s.config.MaxIncomingUniStreams),
 		MaxAckDelay:                     protocol.MaxAckDelayInclGranularity,
 		AckDelayExponent:                protocol.AckDelayExponent,
+		MaxUDPPayloadSize:               protocol.MaxPacketBufferSize,
 		DisableActiveMigration:          true,
 		StatelessResetToken:             &statelessResetToken,
 		OriginalDestinationConnectionID: origDestConnID,
@@ -410,6 +411,7 @@ var newClientConnection = func(
 		MaxBidiStreamNum:               protocol.StreamNum(s.config.MaxIncomingStreams),
 		MaxUniStreamNum:                protocol.StreamNum(s.config.MaxIncomingUniStreams),
 		MaxAckDelay:                    protocol.MaxAckDelayInclGranularity,
+		MaxUDPPayloadSize:              protocol.MaxPacketBufferSize,
 		AckDelayExponent:               protocol.AckDelayExponent,
 		DisableActiveMigration:         true,
 		// For interoperability with quic-go versions before May 2023, this value must be set to a value
diff --git a/internal/wire/transport_parameters.go b/internal/wire/transport_parameters.go
index 216da6ab..cee74b8f 100644
--- a/internal/wire/transport_parameters.go
+++ b/internal/wire/transport_parameters.go
@@ -302,7 +302,7 @@ func (p *TransportParameters) readNumericTransportParameter(b []byte, paramID tr
 		p.MaxIdleTimeout = max(protocol.MinRemoteIdleTimeout, time.Duration(val)*time.Millisecond)
 	case maxUDPPayloadSizeParameterID:
 		if val < 1200 {
-			return fmt.Errorf("invalid value for max_packet_size: %d (minimum 1200)", val)
+			return fmt.Errorf("invalid value for max_udp_payload_size: %d (minimum 1200)", val)
 		}
 		p.MaxUDPPayloadSize = protocol.ByteCount(val)
 	case ackDelayExponentParameterID:
@@ -357,8 +357,10 @@ func (p *TransportParameters) Marshal(pers protocol.Perspective) []byte {
 	b = p.marshalVarintParam(b, initialMaxStreamsUniParameterID, uint64(p.MaxUniStreamNum))
 	// idle_timeout
 	b = p.marshalVarintParam(b, maxIdleTimeoutParameterID, uint64(p.MaxIdleTimeout/time.Millisecond))
-	// max_packet_size
-	b = p.marshalVarintParam(b, maxUDPPayloadSizeParameterID, uint64(protocol.MaxPacketBufferSize))
+	// max_udp_payload_size
+	if p.MaxUDPPayloadSize > 0 {
+		b = p.marshalVarintParam(b, maxUDPPayloadSizeParameterID, uint64(p.MaxUDPPayloadSize))
+	}
 	// max_ack_delay
 	// Only send it if is different from the default value.
 	if p.MaxAckDelay != protocol.DefaultMaxAckDelay {


Test patch:
diff --git a/internal/wire/transport_parameter_test.go b/internal/wire/transport_parameter_test.go
index 721ad62f..4fa9b594 100644
--- a/internal/wire/transport_parameter_test.go
+++ b/internal/wire/transport_parameter_test.go
@@ -28,7 +28,7 @@ func getRandomValueUpTo(max int64) uint64 {
 }
 
 func getRandomValue() uint64 {
-	return getRandomValueUpTo(math.MaxInt64)
+	return getRandomValueUpTo(quicvarint.Max)
 }
 
 var _ = Describe("Transport Parameters", func() {
@@ -102,7 +102,8 @@ var _ = Describe("Transport Parameters", func() {
 			RetrySourceConnectionID:         &rcid,
 			AckDelayExponent:                13,
 			MaxAckDelay:                     42 * time.Millisecond,
-			ActiveConnectionIDLimit:         2 + getRandomValueUpTo(math.MaxInt64-2),
+			ActiveConnectionIDLimit:         2 + getRandomValueUpTo(quicvarint.Max-2),
+			MaxUDPPayloadSize:               1200 + protocol.ByteCount(getRandomValueUpTo(quicvarint.Max-1200)),
 			MaxDatagramFrameSize:            protocol.ByteCount(getRandomValue()),
 		}
 		data := params.Marshal(protocol.PerspectiveServer)
@@ -124,6 +125,7 @@ var _ = Describe("Transport Parameters", func() {
 		Expect(p.AckDelayExponent).To(Equal(uint8(13)))
 		Expect(p.MaxAckDelay).To(Equal(42 * time.Millisecond))
 		Expect(p.ActiveConnectionIDLimit).To(Equal(params.ActiveConnectionIDLimit))
+		Expect(p.MaxUDPPayloadSize).To(Equal(params.MaxUDPPayloadSize))
 		Expect(p.MaxDatagramFrameSize).To(Equal(params.MaxDatagramFrameSize))
 	})
 
@@ -176,13 +178,13 @@ var _ = Describe("Transport Parameters", func() {
 		}))
 	})
 
-	It("errors when the max_packet_size is too small", func() {
+	It("errors when the max_udp_payload_size is too small", func() {
 		b := quicvarint.Append(nil, uint64(maxUDPPayloadSizeParameterID))
 		b = quicvarint.Append(b, uint64(quicvarint.Len(1199)))
 		b = quicvarint.Append(b, 1199)
 		Expect((&TransportParameters{}).Unmarshal(b, protocol.PerspectiveServer)).To(MatchError(&qerr.TransportError{
 			ErrorCode:    qerr.TransportParameterError,
-			ErrorMessage: "invalid value for max_packet_size: 1199 (minimum 1200)",
+			ErrorMessage: "invalid value for max_udp_payload_size: 1199 (minimum 1200)",
 		}))
 	})
 
@@ -499,7 +501,7 @@ var _ = Describe("Transport Parameters", func() {
 				InitialMaxData:                 protocol.ByteCount(getRandomValue()),
 				MaxBidiStreamNum:               protocol.StreamNum(getRandomValueUpTo(int64(protocol.MaxStreamCount))),
 				MaxUniStreamNum:                protocol.StreamNum(getRandomValueUpTo(int64(protocol.MaxStreamCount))),
-				ActiveConnectionIDLimit:        2 + getRandomValueUpTo(math.MaxInt64-2),
+				ActiveConnectionIDLimit:        2 + getRandomValueUpTo(quicvarint.Max-2),
 				MaxDatagramFrameSize:           protocol.ByteCount(getRandomValueUpTo(int64(MaxDatagramSize))),
 			}
 			Expect(params.ValidFor0RTT(params)).To(BeTrue())
@@ -748,7 +750,7 @@ func benchmarkTransportParameters(b *testing.B, withPreferredAddress bool) {
 		RetrySourceConnectionID:         &rcid,
 		AckDelayExponent:                13,
 		MaxAckDelay:                     42 * time.Millisecond,
-		ActiveConnectionIDLimit:         2 + getRandomValueUpTo(math.MaxInt64-2),
+		ActiveConnectionIDLimit:         2 + getRandomValueUpTo(quicvarint.Max-2),
 		MaxDatagramFrameSize:            protocol.ByteCount(getRandomValue()),
 	}
 	var token2 protocol.StatelessResetToken