dict: resolve unknown vendor AVPs to Unknown, not a same-code base AVP

CONTRIBUTORS

@@ -33,6 +33,7 @@ Lucas Fontes <35205+lxfontes@users.noreply.github.com>
 mspronk <martijn@orangemountain.ca>
 Muhammad Talha Khan <raotalha302.rt@gmail.com>
 Oriol Batalla <obatalla@fb.com>
+Patrick Bosch <patrick.d.bosch@gmail.com>
 pmcgleenon <pmcgleenon@users.noreply.github.com>
 rakshasa <sundell.software@gmail.com>
 Sachith Muhandiram <sachith@vizuamatix.com>

diam/avp_test.go

@@ -118,6 +118,66 @@ func TestDecodeAVPWithVendorID(t *testing.T) {
 	}
 }
 
+func TestDecodeAVPUnknownVendor(t *testing.T) {
+	// Unknown vendor AVP decodes as type Unknown with the full payload,
+	// not bound to same-code base NAS-Port.
+	payload := []byte("08-21090003e80000") // 17 bytes, not a 4-byte Unsigned32
+	a := NewAVP(5, avp.Vbit, 10415, datatype.OctetString(payload))
+	b, err := a.Serialize()
+	if err != nil {
+		t.Fatal("Failed to serialize AVP:", err)
+	}
+	got, err := DecodeAVP(b, 4, dict.Default)
+	if err != nil {
+		t.Fatal("Failed to decode unknown vendor AVP:", err)
+	}
+	if got == nil || got.Data == nil {
+		t.Fatal("Expected Unknown AVP with non-nil Data")
+	}
+	if got.Len() != len(b) {
+		t.Fatalf("Unknown AVP Len() %d does not match wire length %d (desync)",
+			got.Len(), len(b))
+	}
+	if u, ok := got.Data.(datatype.Unknown); !ok {
+		t.Fatalf("Expected datatype.Unknown, got %T", got.Data)
+	} else if !bytes.Equal([]byte(u), payload) {
+		t.Fatalf("Unknown payload not preserved: have %x, want %x", []byte(u), payload)
+	}
+}
+
+func TestReadMessageUnknownVendorAVP(t *testing.T) {
+	// Regression: an unknown vendor AVP colliding with base code 5 (NAS-Port) must
+	// parse without desyncing the stream or panicking in (*AVP).Len().
+	m := NewRequest(CreditControl, 4, dict.Default)
+	m.NewAVP(avp.SessionID, avp.Mbit, 0, datatype.UTF8String("test;1;0;1"))
+	m.NewAVP(avp.OriginHost, avp.Mbit, 0, datatype.DiameterIdentity("client"))
+	m.NewAVP(avp.OriginRealm, avp.Mbit, 0, datatype.DiameterIdentity("localhost"))
+	m.NewAVP(avp.CCRequestType, avp.Mbit, 0, datatype.Enumerated(1))
+	m.NewAVP(avp.CCRequestNumber, avp.Mbit, 0, datatype.Unsigned32(0))
+	m.NewAVP(5, avp.Vbit, 10415, datatype.OctetString([]byte("08-21090003e80000")))
+
+	var buf bytes.Buffer
+	if _, err := m.WriteTo(&buf); err != nil {
+		t.Fatal("Failed to serialize message:", err)
+	}
+	wire := buf.Bytes()
+
+	got, err := ReadMessage(bytes.NewReader(wire), dict.Default)
+	if err != nil {
+		t.Fatal("Failed to read message:", err)
+	}
+	if len(got.AVP) != 6 {
+		t.Fatalf("Expected 6 AVPs, got %d (stream desync)", len(got.AVP))
+	}
+	last := got.AVP[len(got.AVP)-1]
+	if last.Code != 5 || last.VendorID != 10415 {
+		t.Fatalf("Last AVP misframed: code=%d vendor=%d", last.Code, last.VendorID)
+	}
+	if last.Data == nil {
+		t.Fatal("Unknown AVP decoded with nil Data")
+	}
+}
+
 func TestEncodeAVP(t *testing.T) {
 	a := &AVP{
 		Code:  avp.OriginHost,

diam/dict/util.go

@@ -142,17 +142,11 @@ retry:
 //
 // FindAVPByCode must never be called concurrently with LoadFile or Load.
 func (p *Parser) FindAVPByCode(appid, code, vendorID uint32) (*AVP, error) {
-	// Primary lookup — covers both app-specific and inherited AVPs
-	// thanks to mergeInheritedAVPs() called at load time.
+	// Exact (appid, code, vendorID) match; inherited AVPs are pre-merged by mergeInheritedAVPs().
+	// An absent vendor AVP resolves to Unknown, never cross-vendor (RFC 6733 §4.1/§11.1.1).
 	if avp, ok := p.avpcode[codeIdx{appid, code, vendorID}]; ok {
 		return avp, nil
 	}
-	// Fallback: if a specific vendorID was given, retry with UndefinedVendorID.
-	if vendorID != UndefinedVendorID {
-		if avp, ok := p.avpcode[codeIdx{appid, code, UndefinedVendorID}]; ok {
-			return avp, nil
-		}
-	}
 	return MakeUnknownAVP(appid, code, vendorID),
 		fmt.Errorf("Could not find AVP %d for Vendor: %d", code, vendorID)
 }

diam/dict/util_test.go

@@ -7,6 +7,8 @@ package dict
 import (
 	"bytes"
 	"testing"
+
+	"github.com/fiorix/go-diameter/v4/diam/datatype"
 )
 
 func TestApps(t *testing.T) {
@@ -172,6 +174,44 @@ func TestFindAVPWithVendorNoInfiniteRecursion(t *testing.T) {
 	}
 }
 
+func TestFindAVPByCode(t *testing.T) {
+	// Exact (appid, code, vendorID) match.
+	if avp, err := Default.FindAVPByCode(4, 461, UndefinedVendorID); err != nil {
+		t.Fatalf("FindAVPByCode error for Service-Context-Id: %v", err)
+	} else if avp.Name != "Service-Context-Id" {
+		t.Fatalf("Unexpected AVP %q, expected Service-Context-Id", avp.Name)
+	}
+
+	// Inherited base AVP (app 4 → base) resolves via the pre-merged index.
+	if avp, err := Default.FindAVPByCode(4, 263, 0); err != nil {
+		t.Fatalf("FindAVPByCode error for inherited Session-Id: %v", err)
+	} else if avp.Name != "Session-Id" {
+		t.Fatalf("Unexpected AVP %q, expected Session-Id", avp.Name)
+	}
+
+	// Inheritance through the full parent chain: Gx (16777238) → 4 → base.
+	if avp, err := Default.FindAVPByCode(16777238, 264, 0); err != nil {
+		t.Fatalf("FindAVPByCode error for inherited Origin-Host: %v", err)
+	} else if avp.Name != "Origin-Host" {
+		t.Fatalf("Unexpected AVP %q, expected Origin-Host", avp.Name)
+	}
+
+	// Unknown vendor AVP resolves to Unknown, not cross-vendor to base NAS-Port (code 5, vendor 0).
+	avp, err := Default.FindAVPByCode(4, 5, 10415)
+	if err == nil {
+		t.Fatal("Expected error for unknown vendor AVP code 5 / vendor 10415")
+	}
+	if avp == nil {
+		t.Fatal("Expected Unknown AVP, got nil")
+	}
+	if avp.Name != "Unknown-5-10415" {
+		t.Fatalf("Expected Unknown-5-10415, got %q (cross-vendor mismatch)", avp.Name)
+	}
+	if avp.Data.Type != datatype.UnknownType {
+		t.Fatalf("Expected Unknown data type, got %v", avp.Data.Type)
+	}
+}
+
 func BenchmarkFindAVPName(b *testing.B) {
 	for n := 0; n < b.N; n++ {
 		Default.FindAVP(0, "Session-Id")