diff --git a/CONTRIBUTORS b/CONTRIBUTORS index f518ee2a..b23675b5 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -33,6 +33,7 @@ Lucas Fontes <35205+lxfontes@users.noreply.github.com> mspronk Muhammad Talha Khan Oriol Batalla +Patrick Bosch pmcgleenon rakshasa Sachith Muhandiram diff --git a/diam/avp_test.go b/diam/avp_test.go index 9dbae3dc..64bdffc5 100644 --- a/diam/avp_test.go +++ b/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, diff --git a/diam/dict/util.go b/diam/dict/util.go index d56622c7..73eb861f 100644 --- a/diam/dict/util.go +++ b/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) } diff --git a/diam/dict/util_test.go b/diam/dict/util_test.go index 5ebd6e19..fbb8bb8c 100644 --- a/diam/dict/util_test.go +++ b/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")