From 30618a31fe37cdf91a868b697a496977b9a39310 Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Tue, 14 Apr 2020 21:11:27 +0700
Subject: [PATCH] fix qlogging of packet headers

---
 qlog/packet_header.go      | 16 +++++++++---
 qlog/packet_header_test.go | 52 ++++++++++++++++++++++++++------------
 2 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/qlog/packet_header.go b/qlog/packet_header.go
index b27d6236b..dc3436295 100644
--- a/qlog/packet_header.go
+++ b/qlog/packet_header.go
@@ -46,6 +46,7 @@ func getPacketTypeFromEncryptionLevel(encLevel protocol.EncryptionLevel) PacketT
 
 func transformHeader(hdr *wire.Header) *packetHeader {
 	return &packetHeader{
+		PacketType:       PacketTypeFromHeader(hdr),
 		PayloadLength:    hdr.Length,
 		SrcConnectionID:  hdr.SrcConnectionID,
 		DestConnectionID: hdr.DestConnectionID,
@@ -60,6 +61,9 @@ func transformExtendedHeader(hdr *wire.ExtendedHeader) *packetHeader {
 }
 
 type packetHeader struct {
+	// We don't log the packet type as a part of the header yet, see https://github.com/quiclog/internet-drafts/issues/40.
+	PacketType PacketType
+
 	PacketNumber  protocol.PacketNumber
 	PayloadLength protocol.ByteCount
 	// Size of the QUIC packet (QUIC header + payload).
@@ -72,18 +76,22 @@ type packetHeader struct {
 }
 
 func (h packetHeader) MarshalJSONObject(enc *gojay.Encoder) {
-	enc.Int64KeyOmitEmpty("packet_number", int64(h.PacketNumber))
+	if h.PacketType != PacketTypeRetry && h.PacketType != PacketTypeVersionNegotiation {
+		enc.Int64Key("packet_number", int64(h.PacketNumber))
+	}
 	enc.Int64KeyOmitEmpty("payload_length", int64(h.PayloadLength))
 	enc.Int64KeyOmitEmpty("packet_size", int64(h.PacketSize))
 	if h.Version != 0 {
 		enc.StringKey("version", versionNumber(h.Version).String())
 	}
-	if h.SrcConnectionID.Len() > 0 {
+	if h.PacketType != PacketType1RTT {
 		enc.IntKey("scil", h.SrcConnectionID.Len())
-		enc.StringKey("scid", connectionID(h.SrcConnectionID).String())
+		if h.SrcConnectionID.Len() > 0 {
+			enc.StringKey("scid", connectionID(h.SrcConnectionID).String())
+		}
 	}
+	enc.IntKey("dcil", h.DestConnectionID.Len())
 	if h.DestConnectionID.Len() > 0 {
-		enc.IntKey("dcil", h.DestConnectionID.Len())
 		enc.StringKey("dcid", connectionID(h.DestConnectionID).String())
 	}
 }
diff --git a/qlog/packet_header_test.go b/qlog/packet_header_test.go
index 05d11142c..5b90a61c1 100644
--- a/qlog/packet_header_test.go
+++ b/qlog/packet_header_test.go
@@ -80,11 +80,12 @@ var _ = Describe("Packet Header", func() {
 			checkEncoding(data, expected)
 		}
 
-		It("marshals a header", func() {
+		It("marshals a header for a 1-RTT packet", func() {
 			check(
 				&wire.ExtendedHeader{PacketNumber: 42},
 				map[string]interface{}{
 					"packet_number": 42,
+					"dcil":          0,
 				},
 			)
 		})
@@ -93,11 +94,38 @@ var _ = Describe("Packet Header", func() {
 			check(
 				&wire.ExtendedHeader{
 					PacketNumber: 42,
-					Header:       wire.Header{Length: 123},
+					Header: wire.Header{
+						IsLongHeader: true,
+						Type:         protocol.PacketTypeInitial,
+						Length:       123,
+						Version:      protocol.VersionNumber(0xdecafbad),
+					},
 				},
 				map[string]interface{}{
 					"packet_number":  42,
 					"payload_length": 123,
+					"dcil":           0,
+					"scil":           0,
+					"version":        "decafbad",
+				},
+			)
+		})
+
+		It("marshals a packet with packet number 0", func() {
+			check(
+				&wire.ExtendedHeader{
+					PacketNumber: 0,
+					Header: wire.Header{
+						IsLongHeader: true,
+						Type:         protocol.PacketTypeHandshake,
+						Version:      protocol.VersionNumber(0xdecafbad),
+					},
+				},
+				map[string]interface{}{
+					"packet_number": 0,
+					"dcil":          0,
+					"scil":          0,
+					"version":       "decafbad",
 				},
 			)
 		})
@@ -107,18 +135,23 @@ var _ = Describe("Packet Header", func() {
 				&wire.ExtendedHeader{
 					PacketNumber: 42,
 					Header: wire.Header{
+						IsLongHeader:    true,
+						Type:            protocol.PacketTypeHandshake,
 						SrcConnectionID: protocol.ConnectionID{0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff},
+						Version:         protocol.VersionNumber(0xdecafbad),
 					},
 				},
 				map[string]interface{}{
 					"packet_number": 42,
+					"dcil":          0,
 					"scil":          16,
 					"scid":          "00112233445566778899aabbccddeeff",
+					"version":       "decafbad",
 				},
 			)
 		})
 
-		It("marshals a header with a destination connection ID", func() {
+		It("marshals a 1-RTT header with a destination connection ID", func() {
 			check(
 				&wire.ExtendedHeader{
 					PacketNumber: 42,
@@ -131,18 +164,5 @@ var _ = Describe("Packet Header", func() {
 				},
 			)
 		})
-
-		It("marshals a header with a version number", func() {
-			check(
-				&wire.ExtendedHeader{
-					PacketNumber: 42,
-					Header:       wire.Header{Version: protocol.VersionNumber(0xdecafbad)},
-				},
-				map[string]interface{}{
-					"packet_number": 42,
-					"version":       "decafbad",
-				},
-			)
-		})
 	})
 })