Repository: quic-go/quic-go
Pull request #4985 (2025-03-15 04:08:14)

wire: correctly handle undefined IPv4 / IPv6 preferred address values
For #4965.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/4985?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 83.97%. Comparing base [(`24acc54`)](https://app.codecov.io/gh/quic-go/quic-go/commit/24acc54ef10777e086574760c54ec27fefb32a4e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`9249731`)](https://app.codecov.io/gh/quic-go/quic-go/commit/9249731c7703ae31067d60a77da05b19c1b85ecd?dropdown=coverage&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    #4985      +/-   ##
==========================================
- Coverage   84.09%   83.97%   -0.13%     
==========================================
  Files         153      150       -3     
  Lines       16848    16514     -334     
==========================================
- Hits        14168    13866     -302     
+ Misses       2149     2131      -18     
+ Partials      531      517      -14     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/4985?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).

<details><summary>🚀 New features to boost your workflow: </summary>

- ❄ [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/internal/wire/transport_parameters.go b/internal/wire/transport_parameters.go
index cee74b8f..d39189a0 100644
--- a/internal/wire/transport_parameters.go
+++ b/internal/wire/transport_parameters.go
@@ -245,11 +245,15 @@ func (p *TransportParameters) readPreferredAddress(b []byte, expectedLen int) er
 	copy(ipv4[:], b[:4])
 	port4 := binary.BigEndian.Uint16(b[4:])
 	b = b[4+2:]
-	pa.IPv4 = netip.AddrPortFrom(netip.AddrFrom4(ipv4), port4)
+	if port4 != 0 && ipv4 != [4]byte{} {
+		pa.IPv4 = netip.AddrPortFrom(netip.AddrFrom4(ipv4), port4)
+	}
 	var ipv6 [16]byte
 	copy(ipv6[:], b[:16])
 	port6 := binary.BigEndian.Uint16(b[16:])
-	pa.IPv6 = netip.AddrPortFrom(netip.AddrFrom16(ipv6), port6)
+	if port6 != 0 && ipv6 != [16]byte{} {
+		pa.IPv6 = netip.AddrPortFrom(netip.AddrFrom16(ipv6), port6)
+	}
 	b = b[16+2:]
 	connIDLen := int(b[0])
 	b = b[1:]
@@ -391,12 +395,20 @@ func (p *TransportParameters) Marshal(pers protocol.Perspective) []byte {
 		if p.PreferredAddress != nil {
 			b = quicvarint.Append(b, uint64(preferredAddressParameterID))
 			b = quicvarint.Append(b, 4+2+16+2+1+uint64(p.PreferredAddress.ConnectionID.Len())+16)
-			ip4 := p.PreferredAddress.IPv4.Addr().As4()
-			b = append(b, ip4[:]...)
-			b = binary.BigEndian.AppendUint16(b, p.PreferredAddress.IPv4.Port())
-			ip6 := p.PreferredAddress.IPv6.Addr().As16()
-			b = append(b, ip6[:]...)
-			b = binary.BigEndian.AppendUint16(b, p.PreferredAddress.IPv6.Port())
+			if p.PreferredAddress.IPv4.IsValid() {
+				ipv4 := p.PreferredAddress.IPv4.Addr().As4()
+				b = append(b, ipv4[:]...)
+				b = binary.BigEndian.AppendUint16(b, p.PreferredAddress.IPv4.Port())
+			} else {
+				b = append(b, make([]byte, 6)...)
+			}
+			if p.PreferredAddress.IPv6.IsValid() {
+				ipv6 := p.PreferredAddress.IPv6.Addr().As16()
+				b = append(b, ipv6[:]...)
+				b = binary.BigEndian.AppendUint16(b, p.PreferredAddress.IPv6.Port())
+			} else {
+				b = append(b, make([]byte, 18)...)
+			}
 			b = append(b, uint8(p.PreferredAddress.ConnectionID.Len()))
 			b = append(b, p.PreferredAddress.ConnectionID.Bytes()...)
 			b = append(b, p.PreferredAddress.StatelessResetToken[:]...)


Test patch:
diff --git a/internal/wire/transport_parameter_test.go b/internal/wire/transport_parameter_test.go
index f5ecb873..d94b857d 100644
--- a/internal/wire/transport_parameter_test.go
+++ b/internal/wire/transport_parameter_test.go
@@ -436,13 +436,38 @@ func TestTransportParameterRejectsDuplicateParameters(t *testing.T) {
 	require.Equal(t, fmt.Sprintf("received duplicate transport parameter %#x", initialMaxStreamDataBidiLocalParameterID), transportErr.ErrorMessage)
 }
 
-func TestPreferredAddressMarshalAndUnmarshal(t *testing.T) {
+func TestTransportParameterPreferredAddress(t *testing.T) {
+	testCases := []struct {
+		name    string
+		hasIPv4 bool
+		hasIPv6 bool
+	}{
+		{"IPv4 and IPv6", true, true},
+		{"IPv4 only", true, false},
+		{"IPv6 only", false, true},
+		{"neither IPv4 nor IPv6", false, false},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			testTransportParameterPreferredAddress(t, tc.hasIPv4, tc.hasIPv6)
+		})
+	}
+}
+
+func testTransportParameterPreferredAddress(t *testing.T, hasIPv4, hasIPv6 bool) {
+	addr4 := netip.AddrPortFrom(netip.AddrFrom4([4]byte{127, 0, 0, 1}), 42)
+	addr6 := netip.AddrPortFrom(netip.AddrFrom16([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}), 13)
 	pa := &PreferredAddress{
-		IPv4:                netip.AddrPortFrom(netip.AddrFrom4([4]byte{127, 0, 0, 1}), 42),
-		IPv6:                netip.AddrPortFrom(netip.AddrFrom16([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}), 13),
 		ConnectionID:        protocol.ParseConnectionID([]byte{0xde, 0xad, 0xbe, 0xef}),
 		StatelessResetToken: protocol.StatelessResetToken{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1},
 	}
+	if hasIPv4 {
+		pa.IPv4 = addr4
+	}
+	if hasIPv6 {
+		pa.IPv6 = addr6
+	}
 
 	data := (&TransportParameters{
 		PreferredAddress:        pa,
@@ -450,15 +475,24 @@ func TestPreferredAddressMarshalAndUnmarshal(t *testing.T) {
 		ActiveConnectionIDLimit: 2,
 	}).Marshal(protocol.PerspectiveServer)
 	p := &TransportParameters{}
-	err := p.Unmarshal(data, protocol.PerspectiveServer)
-	require.NoError(t, err)
-	require.Equal(t, pa.IPv4, p.PreferredAddress.IPv4)
-	require.Equal(t, pa.IPv6, p.PreferredAddress.IPv6)
+	require.NoError(t, p.Unmarshal(data, protocol.PerspectiveServer))
+	if hasIPv4 {
+		require.True(t, p.PreferredAddress.IPv4.IsValid())
+		require.Equal(t, addr4, p.PreferredAddress.IPv4)
+	} else {
+		require.False(t, p.PreferredAddress.IPv4.IsValid())
+	}
+	if hasIPv6 {
+		require.True(t, p.PreferredAddress.IPv6.IsValid())
+		require.Equal(t, addr6, p.PreferredAddress.IPv6)
+	} else {
+		require.False(t, p.PreferredAddress.IPv6.IsValid())
+	}
 	require.Equal(t, pa.ConnectionID, p.PreferredAddress.ConnectionID)
 	require.Equal(t, pa.StatelessResetToken, p.PreferredAddress.StatelessResetToken)
 }
 
-func TestPreferredAddressFromClient(t *testing.T) {
+func TestTransportParameterPreferredAddressFromClient(t *testing.T) {
 	b := quicvarint.Append(nil, uint64(preferredAddressParameterID))
 	b = quicvarint.Append(b, 6)
 	b = append(b, []byte("foobar")...)
@@ -471,7 +505,7 @@ func TestPreferredAddressFromClient(t *testing.T) {
 	require.Equal(t, "client sent a preferred_address", transportErr.ErrorMessage)
 }
 
-func TestPreferredAddressZeroLengthConnectionID(t *testing.T) {
+func TestTransportParameterPreferredAddressZeroLengthConnectionID(t *testing.T) {
 	pa := &PreferredAddress{
 		IPv4:                netip.AddrPortFrom(netip.AddrFrom4([4]byte{127, 0, 0, 1}), 42),
 		IPv6:                netip.AddrPortFrom(netip.AddrFrom16([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}), 13),