From b541794d84c4208259d8c7778fd98ad48a5f8272 Mon Sep 17 00:00:00 2001 From: Denozordec Date: Mon, 6 Apr 2026 23:33:54 +0700 Subject: [PATCH] feat: enhance community attribute handling in BIRD format. Add validation for community route bodies to ensure correct tuple structure and introduce tests for single-element standard arrays, improving error handling and robustness in community attribute processing. --- internal/birdfmt/community_bird.go | 68 ++++++++++++++++++++++--- internal/birdfmt/community_bird_test.go | 7 +++ 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/internal/birdfmt/community_bird.go b/internal/birdfmt/community_bird.go index f0ed1b7..329ee3e 100644 --- a/internal/birdfmt/community_bird.go +++ b/internal/birdfmt/community_bird.go @@ -10,6 +10,10 @@ import ( var standardPairRE = regexp.MustCompile(`^\s*(\d{1,5}):(\d{1,5})\s*$`) +// badCommunityAddRE matches invalid BIRD filter calls like bgp_community.add(65000,1) +// (first argument is an int). Valid form is bgp_community.add((65000,1)); — note the pair tuple. +var badCommunityAddRE = regexp.MustCompile(`\b(?:bgp_community|bgp_large_community)\.add\(\s*\d`) + // RouteCommunityAttrs returns BIRD 2 lines (indented with 4 spaces) for use inside a static `route … { … }` block. // Empty string means no community attributes. func RouteCommunityAttrs(kind, name, valueJSON string) (string, error) { @@ -20,26 +24,46 @@ func RouteCommunityAttrs(kind, name, valueJSON string) (string, error) { } if s, ok := parseLargeFromJSONLoose(raw); ok { + if err := validateCommunityRouteBody(s); err != nil { + return "", err + } return s, nil } _ = kind // reserved for future kinds (e.g. extended communities) if m := standardPairRE.FindStringSubmatch(name); len(m) == 3 { - return standardAddLine(m[1], m[2]) + s, err := standardAddLine(m[1], m[2]) + if err != nil { + return "", err + } + return s, validateCommunityRouteBody(s) } var obj map[string]any if json.Unmarshal([]byte(raw), &obj) == nil { if v, ok := obj["standard"].(string); ok { if m := standardPairRE.FindStringSubmatch(v); len(m) == 3 { - return standardAddLine(m[1], m[2]) + s, err := standardAddLine(m[1], m[2]) + if err != nil { + return "", err + } + return s, validateCommunityRouteBody(s) } } - if arr, ok := obj["standard"].([]any); ok && len(arr) == 2 { - a, aok := numberToUint32String(arr[0]) - b, bok := numberToUint32String(arr[1]) - if aok && bok { - return standardAddLine(a, b) + if arr, ok := obj["standard"].([]any); ok { + if len(arr) == 1 { + return "", fmt.Errorf(`birdfmt: value_json "standard" must be a pair [a,b] or "a:b" string, got single element`) + } + if len(arr) == 2 { + a, aok := numberToUint32String(arr[0]) + b, bok := numberToUint32String(arr[1]) + if aok && bok { + s, err := standardAddLine(a, b) + if err != nil { + return "", err + } + return s, validateCommunityRouteBody(s) + } } } } @@ -47,13 +71,36 @@ func RouteCommunityAttrs(kind, name, valueJSON string) (string, error) { var strVal string if json.Unmarshal([]byte(raw), &strVal) == nil { if m := standardPairRE.FindStringSubmatch(strVal); len(m) == 3 { - return standardAddLine(m[1], m[2]) + s, err := standardAddLine(m[1], m[2]) + if err != nil { + return "", err + } + return s, validateCommunityRouteBody(s) } } return "", nil } +// validateCommunityRouteBody rejects lines that would make BIRD 2 fail parse-check with +// "Argument 1 of 'add' expected clist / quad / ip / pair, got int" (wrong add() shape). +func validateCommunityRouteBody(body string) error { + body = strings.TrimSpace(body) + if body == "" { + return nil + } + for _, line := range strings.Split(body, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + if badCommunityAddRE.MatchString(line) { + return fmt.Errorf("birdfmt: invalid community line (expected pair/triple tuple inside add(), e.g. bgp_community.add((65000,1));): %q", line) + } + } + return nil +} + func standardAddLine(a, b string) (string, error) { ai, err1 := strconv.ParseUint(strings.TrimSpace(a), 10, 16) bi, err2 := strconv.ParseUint(strings.TrimSpace(b), 10, 16) @@ -111,6 +158,11 @@ func numberToUint32String(v any) (string, bool) { return "", false } return strconv.FormatUint(uint64(x), 10), true + case int64: + if x < 0 || x > 4294967295 { + return "", false + } + return strconv.FormatUint(uint64(x), 10), true case string: n, err := strconv.ParseUint(strings.TrimSpace(x), 10, 32) if err != nil { diff --git a/internal/birdfmt/community_bird_test.go b/internal/birdfmt/community_bird_test.go index f775c3b..84f66d7 100644 --- a/internal/birdfmt/community_bird_test.go +++ b/internal/birdfmt/community_bird_test.go @@ -21,3 +21,10 @@ func TestRouteCommunityAttrs_LargeJSON(t *testing.T) { t.Fatalf("got %q", s) } } + +func TestRouteCommunityAttrs_StandardArrayOneElement(t *testing.T) { + _, err := RouteCommunityAttrs("", "x", `{"standard":[65000]}`) + if err == nil { + t.Fatal("expected error for single-element standard pair") + } +}