silly optimization

header/header.go

@@ -190,13 +190,18 @@ func SubTypeName(t MessageType, s MessageSubType) string {
 }
 
 func IsValidSubType(t MessageType, s MessageSubType) bool {
-	if n, ok := subTypeMap[t]; ok {
-		if _, ok := (*n)[s]; ok {
-			return true
-		}
+	switch t {
+	case Message:
+		return s == MessageNone || s == MessageRelay
+	case Handshake:
+		return s == HandshakeIXPSK0
+	case Test:
+		return s == TestReply || s == TestRequest
+	case Control, CloseTunnel, RecvError, LightHouse:
+		return s == 0
+	default:
+		return false
 	}
-
-	return false
 }
 
 // NewHeader turns bytes into a header

header/header_test.go

@@ -102,6 +102,57 @@ func TestTypeMap(t *testing.T) {
 	}, subTypeMap)
 }
 
+// mapIsValidSubType is the pre-refactor, map-driven definition of a valid
+// subtype. IsValidSubType was reimplemented as an explicit switch; this keeps
+// the original behavior around so we can prove the switch is equivalent to it.
+func mapIsValidSubType(t MessageType, s MessageSubType) bool {
+	if n, ok := subTypeMap[t]; ok {
+		if _, ok := (*n)[s]; ok {
+			return true
+		}
+	}
+	return false
+}
+
+func TestIsValidSubType(t *testing.T) {
+	// Explicit intent table: documents exactly which subtypes are valid so the
+	// test stays meaningful even if both the switch and subTypeMap change.
+	assert.True(t, IsValidSubType(Message, MessageNone))
+	assert.True(t, IsValidSubType(Message, MessageRelay))
+	assert.False(t, IsValidSubType(Message, 2))
+
+	assert.True(t, IsValidSubType(Handshake, HandshakeIXPSK0))
+	// HandshakeXXPSK0 is defined but not a wire-valid subtype.
+	assert.False(t, IsValidSubType(Handshake, HandshakeXXPSK0))
+
+	assert.True(t, IsValidSubType(Test, TestRequest))
+	assert.True(t, IsValidSubType(Test, TestReply))
+	assert.False(t, IsValidSubType(Test, 2))
+
+	// These types only ever carry subtype 0.
+	for _, mt := range []MessageType{Control, CloseTunnel, RecvError, LightHouse} {
+		assert.True(t, IsValidSubType(mt, 0), "type %d subtype 0 should be valid", mt)
+		assert.False(t, IsValidSubType(mt, 1), "type %d subtype 1 should be invalid", mt)
+	}
+
+	// Unknown/unassigned types are never valid.
+	assert.False(t, IsValidSubType(99, 0))
+
+	// Exhaustive proof of equivalence with the original map-driven logic across
+	// the entire (type, subtype) input space.
+	for ti := 0; ti <= 0xff; ti++ {
+		for si := 0; si <= 0xff; si++ {
+			mt, mst := MessageType(ti), MessageSubType(si)
+			assert.Equalf(t, mapIsValidSubType(mt, mst), IsValidSubType(mt, mst),
+				"IsValidSubType(%d, %d) diverged from map-driven definition", ti, si)
+		}
+	}
+
+	// H method must delegate to the package function.
+	assert.True(t, (&H{Type: Test, Subtype: TestReply}).IsValidSubType())
+	assert.False(t, (&H{Type: Handshake, Subtype: HandshakeXXPSK0}).IsValidSubType())
+}
+
 func TestHeader_String(t *testing.T) {
 	assert.Equal(
 		t,