fix(config): bound output offsets and tidy relation warning tests
- commit
- 43b7e3627d04d976da279b08d783c04a71b7e0bd
- author
- brice · · 2026-10-03 14:05 UTC
- parent
- 29fcc7b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +45 -31
M internal/adapters/config/config.go +10 -2
| @@ -224,6 +224,10 @@ func Load(path string) (ports.Config, []Warning, error) { | |||
| 224 | 224 | return Parse(f) | |
| 225 | 225 | } | |
| 226 | 226 | ||
| 227 | + | // maxOutputOffset bounds output.<name>.offset (logical px), far beyond any | |
| 228 | + | // real layout but small enough that placement arithmetic cannot overflow. | |
| 229 | + | const maxOutputOffset = 65536 | |
| 230 | + | ||
| 227 | 231 | // outputRelation splits "<output>.right-of" (left-of, above, below) into the | |
| 228 | 232 | // relation and the output name. | |
| 229 | 233 | func outputRelation(name string) (ports.OutputRelation, string, bool) { | |
| @@ -384,8 +388,8 @@ func parse(r io.Reader) (ports.Config, map[string]string, []Warning, error) { | |||
| 384 | 388 | if base, ok := strings.CutSuffix(name, ".offset"); ok { | |
| 385 | 389 | name = base | |
| 386 | 390 | off, err := strconv.Atoi(value) | |
| 387 | - | if err != nil { | |
| 388 | - | warn("%s: must be an integer", key) | |
| 391 | + | if err != nil || off < -maxOutputOffset || off > maxOutputOffset { | |
| 392 | + | warn("%s: must be an integer between -%d and %d", key, maxOutputOffset, maxOutputOffset) | |
| 389 | 393 | continue | |
| 390 | 394 | } | |
| 391 | 395 | override() | |
| @@ -406,6 +410,10 @@ func parse(r io.Reader) (ports.Config, map[string]string, []Warning, error) { | |||
| 406 | 410 | if prev, ok := relKeys[name]; ok && prev != key { | |
| 407 | 411 | warn("%s: overrides line %d (%s)", key, seen[prev], prev) | |
| 408 | 412 | delete(raw, prev) | |
| 413 | + | // Stale lines of the other relation keys must not trigger a | |
| 414 | + | // second override warning from override() or a later switch. | |
| 415 | + | delete(seen, prev) | |
| 416 | + | delete(seen, key) | |
| 409 | 417 | } | |
| 410 | 418 | override() | |
| 411 | 419 | relKeys[name] = key | |
| @@ -224,6 +224,10 @@ func Load(path string) (ports.Config, []Warning, error) { | |||||
| 224 | return Parse(f) | 224 | return Parse(f) | ||
| 225 | } | 225 | } | ||
| 226 | 226 | ||||
| 227 | + | // maxOutputOffset bounds output.<name>.offset (logical px), far beyond any | |||
| 228 | + | // real layout but small enough that placement arithmetic cannot overflow. | |||
| 229 | + | const maxOutputOffset = 65536 | |||
| 230 | + | ||||
| 227 | // outputRelation splits "<output>.right-of" (left-of, above, below) into the | 231 | // outputRelation splits "<output>.right-of" (left-of, above, below) into the | ||
| 228 | // relation and the output name. | 232 | // relation and the output name. | ||
| 229 | func outputRelation(name string) (ports.OutputRelation, string, bool) { | 233 | func outputRelation(name string) (ports.OutputRelation, string, bool) { | ||
| @@ -384,8 +388,8 @@ func parse(r io.Reader) (ports.Config, map[string]string, []Warning, error) { | |||||
| 384 | if base, ok := strings.CutSuffix(name, ".offset"); ok { | 388 | if base, ok := strings.CutSuffix(name, ".offset"); ok { | ||
| 385 | name = base | 389 | name = base | ||
| 386 | off, err := strconv.Atoi(value) | 390 | off, err := strconv.Atoi(value) | ||
| 387 | - | if err != nil { | 391 | + | if err != nil || off < -maxOutputOffset || off > maxOutputOffset { |
| 388 | - | warn("%s: must be an integer", key) | 392 | + | warn("%s: must be an integer between -%d and %d", key, maxOutputOffset, maxOutputOffset) |
| 389 | continue | 393 | continue | ||
| 390 | } | 394 | } | ||
| 391 | override() | 395 | override() | ||
| @@ -406,6 +410,10 @@ func parse(r io.Reader) (ports.Config, map[string]string, []Warning, error) { | |||||
| 406 | if prev, ok := relKeys[name]; ok && prev != key { | 410 | if prev, ok := relKeys[name]; ok && prev != key { | ||
| 407 | warn("%s: overrides line %d (%s)", key, seen[prev], prev) | 411 | warn("%s: overrides line %d (%s)", key, seen[prev], prev) | ||
| 408 | delete(raw, prev) | 412 | delete(raw, prev) | ||
| 413 | + | // Stale lines of the other relation keys must not trigger a | |||
| 414 | + | // second override warning from override() or a later switch. | |||
| 415 | + | delete(seen, prev) | |||
| 416 | + | delete(seen, key) | |||
| 409 | } | 417 | } | ||
| 410 | override() | 418 | override() | ||
| 411 | relKeys[name] = key | 419 | relKeys[name] = key | ||
M internal/adapters/config/config_test.go +35 -29
| @@ -651,42 +651,37 @@ func TestOutputRelationOverride(t *testing.T) { | |||
| 651 | 651 | } | |
| 652 | 652 | ||
| 653 | 653 | func TestOutputRelationWarnings(t *testing.T) { | |
| 654 | + | const rangeMsg = "must be an integer between -65536 and 65536" | |
| 655 | + | rel := "output.DP-2.right-of = DP-1\n" | |
| 654 | 656 | cases := []struct { | |
| 655 | - | name, text, want string | |
| 656 | - | line int | |
| 657 | - | anchored []string // outputs that keep a relation | |
| 657 | + | name, text string | |
| 658 | + | want []string // expected warning messages (substring), in order | |
| 659 | + | lines []int // expected warning lines, in order | |
| 660 | + | anchored []string // outputs that keep a relation | |
| 658 | 661 | }{ | |
| 659 | - | {"self", "output.DP-2.right-of = DP-2", "relative to itself", 1, nil}, | |
| 660 | - | {"empty", "output.DP-2.above =", "expected an output name", 1, nil}, | |
| 661 | - | {"bad offset", "output.DP-2.right-of = DP-1\noutput.DP-2.offset = abc", "must be an integer", 2, []string{"DP-2"}}, | |
| 662 | - | {"offset alone", "output.DP-2.offset = 10", "without a relation", 1, nil}, | |
| 663 | - | {"two cycle", "output.A.right-of = B\noutput.B.below = A", "relation cycle", 1, nil}, | |
| 664 | - | {"three cycle", "output.A.right-of = B\noutput.B.right-of = C\noutput.C.above = A", "relation cycle", 1, nil}, | |
| 665 | - | {"tail into cycle", "output.A.right-of = B\noutput.B.right-of = A\noutput.C.left-of = A", "relation cycle", 1, []string{"C"}}, | |
| 662 | + | {"self", "output.DP-2.right-of = DP-2", []string{"relative to itself"}, []int{1}, nil}, | |
| 663 | + | {"empty", "output.DP-2.above =", []string{"expected an output name"}, []int{1}, nil}, | |
| 664 | + | {"bad offset", rel + "output.DP-2.offset = abc", []string{rangeMsg}, []int{2}, []string{"DP-2"}}, | |
| 665 | + | {"offset too large", rel + "output.DP-2.offset = 65537", []string{rangeMsg}, []int{2}, []string{"DP-2"}}, | |
| 666 | + | {"offset too small", rel + "output.DP-2.offset = -65537", []string{rangeMsg}, []int{2}, []string{"DP-2"}}, | |
| 667 | + | {"offset overflow", rel + "output.DP-2.offset = 99999999999999999999", []string{rangeMsg}, []int{2}, []string{"DP-2"}}, | |
| 668 | + | {"offset alone", "output.DP-2.offset = 10", []string{"without a relation"}, []int{1}, nil}, | |
| 669 | + | {"two cycle", "output.A.right-of = B\noutput.B.below = A", []string{"relation cycle", "relation cycle"}, []int{1, 2}, nil}, | |
| 670 | + | {"three cycle", "output.A.right-of = B\noutput.B.right-of = C\noutput.C.above = A", []string{"relation cycle", "relation cycle", "relation cycle"}, []int{1, 2, 3}, nil}, | |
| 671 | + | {"tail into cycle", "output.A.right-of = B\noutput.B.right-of = A\noutput.C.left-of = A", []string{"relation cycle", "relation cycle"}, []int{1, 2}, []string{"C"}}, | |
| 672 | + | {"relation overridden twice", "output.DP-2.right-of = A\noutput.DP-2.below = B\noutput.DP-2.right-of = C", []string{"overrides line 1", "overrides line 2"}, []int{2, 3}, []string{"DP-2"}}, | |
| 673 | + | {"relation overridden three times", "output.DP-2.right-of = A\noutput.DP-2.below = B\noutput.DP-2.right-of = C\noutput.DP-2.below = D", []string{"overrides line 1", "overrides line 2", "overrides line 3"}, []int{2, 3, 4}, []string{"DP-2"}}, | |
| 674 | + | {"same relation twice", "output.DP-2.below = A\noutput.DP-2.below = B", []string{"overrides line 1"}, []int{2}, []string{"DP-2"}}, | |
| 666 | 675 | } | |
| 667 | 676 | for _, tc := range cases { | |
| 668 | 677 | t.Run(tc.name, func(t *testing.T) { | |
| 669 | 678 | c, w := parseString(t, tc.text) | |
| 670 | - | if len(w) == 0 || w[len(w)-1].Line == 0 { | |
| 671 | - | t.Fatalf("warnings %v", w) | |
| 672 | - | } | |
| 673 | - | n := 0 | |
| 674 | - | for _, x := range w { | |
| 675 | - | if strings.Contains(x.Msg, tc.want) { | |
| 676 | - | n++ | |
| 677 | - | } | |
| 678 | - | } | |
| 679 | - | cycle := strings.Contains(tc.want, "cycle") | |
| 680 | - | if n == 0 || (!cycle && len(w) != 1) || w[0].Line != tc.line { | |
| 681 | - | t.Fatalf("warnings %v, want %q at line %d", w, tc.want, tc.line) | |
| 679 | + | if len(w) != len(tc.want) { | |
| 680 | + | t.Fatalf("warnings %v, want %q", w, tc.want) | |
| 682 | 681 | } | |
| 683 | - | if cycle { | |
| 684 | - | members := strings.Count(tc.text, "\n") + 1 | |
| 685 | - | if tc.name == "tail into cycle" { | |
| 686 | - | members = 2 | |
| 687 | - | } | |
| 688 | - | if n != members { | |
| 689 | - | t.Fatalf("%d cycle warnings, want %d: %v", n, members, w) | |
| 682 | + | for i, x := range w { | |
| 683 | + | if !strings.Contains(x.Msg, tc.want[i]) || x.Line != tc.lines[i] { | |
| 684 | + | t.Fatalf("warning %d = %+v, want %q at line %d (all: %v)", i, x, tc.want[i], tc.lines[i], w) | |
| 690 | 685 | } | |
| 691 | 686 | } | |
| 692 | 687 | var got []string | |
| @@ -702,6 +697,17 @@ func TestOutputRelationWarnings(t *testing.T) { | |||
| 702 | 697 | } | |
| 703 | 698 | } | |
| 704 | 699 | ||
| 700 | + | func TestOutputOffsetKeepsEarlierValueWhenInvalid(t *testing.T) { | |
| 701 | + | c, _ := parseString(t, "output.DP-2.right-of = DP-1\noutput.DP-2.offset = 40\noutput.DP-2.offset = 65537\n") | |
| 702 | + | if got := c.Outputs[0].Anchor.Offset; got != 40 { | |
| 703 | + | t.Fatalf("offset %d, want 40", got) | |
| 704 | + | } | |
| 705 | + | c, w := parseString(t, "output.DP-2.right-of = DP-1\noutput.DP-2.offset = 65536\noutput.DP-2.offset = -65536\n") | |
| 706 | + | if len(w) != 1 || c.Outputs[0].Anchor.Offset != -65536 { | |
| 707 | + | t.Fatalf("%v %+v", w, c.Outputs) | |
| 708 | + | } | |
| 709 | + | } | |
| 710 | + | ||
| 705 | 711 | func TestOutputModeLineKeepsAnchor(t *testing.T) { | |
| 706 | 712 | c, w := parseString(t, "output.DP-2.right-of = DP-1\noutput.DP-2.offset = 50\noutput.DP-2 = 1920x1080\n") | |
| 707 | 713 | if len(w) != 0 { | |
| @@ -651,42 +651,37 @@ func TestOutputRelationOverride(t *testing.T) { | |||||
| 651 | } | 651 | } | ||
| 652 | 652 | ||||
| 653 | func TestOutputRelationWarnings(t *testing.T) { | 653 | func TestOutputRelationWarnings(t *testing.T) { | ||
| 654 | + | const rangeMsg = "must be an integer between -65536 and 65536" | |||
| 655 | + | rel := "output.DP-2.right-of = DP-1\n" | |||
| 654 | cases := []struct { | 656 | cases := []struct { | ||
| 655 | - | name, text, want string | 657 | + | name, text string |
| 656 | - | line int | 658 | + | want []string // expected warning messages (substring), in order |
| 657 | - | anchored []string // outputs that keep a relation | 659 | + | lines []int // expected warning lines, in order |
| 660 | + | anchored []string // outputs that keep a relation | |||
| 658 | }{ | 661 | }{ | ||
| 659 | - | {"self", "output.DP-2.right-of = DP-2", "relative to itself", 1, nil}, | 662 | + | {"self", "output.DP-2.right-of = DP-2", []string{"relative to itself"}, []int{1}, nil}, |
| 660 | - | {"empty", "output.DP-2.above =", "expected an output name", 1, nil}, | 663 | + | {"empty", "output.DP-2.above =", []string{"expected an output name"}, []int{1}, nil}, |
| 661 | - | {"bad offset", "output.DP-2.right-of = DP-1\noutput.DP-2.offset = abc", "must be an integer", 2, []string{"DP-2"}}, | 664 | + | {"bad offset", rel + "output.DP-2.offset = abc", []string{rangeMsg}, []int{2}, []string{"DP-2"}}, |
| 662 | - | {"offset alone", "output.DP-2.offset = 10", "without a relation", 1, nil}, | 665 | + | {"offset too large", rel + "output.DP-2.offset = 65537", []string{rangeMsg}, []int{2}, []string{"DP-2"}}, |
| 663 | - | {"two cycle", "output.A.right-of = B\noutput.B.below = A", "relation cycle", 1, nil}, | 666 | + | {"offset too small", rel + "output.DP-2.offset = -65537", []string{rangeMsg}, []int{2}, []string{"DP-2"}}, |
| 664 | - | {"three cycle", "output.A.right-of = B\noutput.B.right-of = C\noutput.C.above = A", "relation cycle", 1, nil}, | 667 | + | {"offset overflow", rel + "output.DP-2.offset = 99999999999999999999", []string{rangeMsg}, []int{2}, []string{"DP-2"}}, |
| 665 | - | {"tail into cycle", "output.A.right-of = B\noutput.B.right-of = A\noutput.C.left-of = A", "relation cycle", 1, []string{"C"}}, | 668 | + | {"offset alone", "output.DP-2.offset = 10", []string{"without a relation"}, []int{1}, nil}, |
| 669 | + | {"two cycle", "output.A.right-of = B\noutput.B.below = A", []string{"relation cycle", "relation cycle"}, []int{1, 2}, nil}, | |||
| 670 | + | {"three cycle", "output.A.right-of = B\noutput.B.right-of = C\noutput.C.above = A", []string{"relation cycle", "relation cycle", "relation cycle"}, []int{1, 2, 3}, nil}, | |||
| 671 | + | {"tail into cycle", "output.A.right-of = B\noutput.B.right-of = A\noutput.C.left-of = A", []string{"relation cycle", "relation cycle"}, []int{1, 2}, []string{"C"}}, | |||
| 672 | + | {"relation overridden twice", "output.DP-2.right-of = A\noutput.DP-2.below = B\noutput.DP-2.right-of = C", []string{"overrides line 1", "overrides line 2"}, []int{2, 3}, []string{"DP-2"}}, | |||
| 673 | + | {"relation overridden three times", "output.DP-2.right-of = A\noutput.DP-2.below = B\noutput.DP-2.right-of = C\noutput.DP-2.below = D", []string{"overrides line 1", "overrides line 2", "overrides line 3"}, []int{2, 3, 4}, []string{"DP-2"}}, | |||
| 674 | + | {"same relation twice", "output.DP-2.below = A\noutput.DP-2.below = B", []string{"overrides line 1"}, []int{2}, []string{"DP-2"}}, | |||
| 666 | } | 675 | } | ||
| 667 | for _, tc := range cases { | 676 | for _, tc := range cases { | ||
| 668 | t.Run(tc.name, func(t *testing.T) { | 677 | t.Run(tc.name, func(t *testing.T) { | ||
| 669 | c, w := parseString(t, tc.text) | 678 | c, w := parseString(t, tc.text) | ||
| 670 | - | if len(w) == 0 || w[len(w)-1].Line == 0 { | 679 | + | if len(w) != len(tc.want) { |
| 671 | - | t.Fatalf("warnings %v", w) | 680 | + | t.Fatalf("warnings %v, want %q", w, tc.want) |
| 672 | - | } | |||
| 673 | - | n := 0 | |||
| 674 | - | for _, x := range w { | |||
| 675 | - | if strings.Contains(x.Msg, tc.want) { | |||
| 676 | - | n++ | |||
| 677 | - | } | |||
| 678 | - | } | |||
| 679 | - | cycle := strings.Contains(tc.want, "cycle") | |||
| 680 | - | if n == 0 || (!cycle && len(w) != 1) || w[0].Line != tc.line { | |||
| 681 | - | t.Fatalf("warnings %v, want %q at line %d", w, tc.want, tc.line) | |||
| 682 | } | 681 | } | ||
| 683 | - | if cycle { | 682 | + | for i, x := range w { |
| 684 | - | members := strings.Count(tc.text, "\n") + 1 | 683 | + | if !strings.Contains(x.Msg, tc.want[i]) || x.Line != tc.lines[i] { |
| 685 | - | if tc.name == "tail into cycle" { | 684 | + | t.Fatalf("warning %d = %+v, want %q at line %d (all: %v)", i, x, tc.want[i], tc.lines[i], w) |
| 686 | - | members = 2 | |||
| 687 | - | } | |||
| 688 | - | if n != members { | |||
| 689 | - | t.Fatalf("%d cycle warnings, want %d: %v", n, members, w) | |||
| 690 | } | 685 | } | ||
| 691 | } | 686 | } | ||
| 692 | var got []string | 687 | var got []string | ||
| @@ -702,6 +697,17 @@ func TestOutputRelationWarnings(t *testing.T) { | |||||
| 702 | } | 697 | } | ||
| 703 | } | 698 | } | ||
| 704 | 699 | ||||
| 700 | + | func TestOutputOffsetKeepsEarlierValueWhenInvalid(t *testing.T) { | |||
| 701 | + | c, _ := parseString(t, "output.DP-2.right-of = DP-1\noutput.DP-2.offset = 40\noutput.DP-2.offset = 65537\n") | |||
| 702 | + | if got := c.Outputs[0].Anchor.Offset; got != 40 { | |||
| 703 | + | t.Fatalf("offset %d, want 40", got) | |||
| 704 | + | } | |||
| 705 | + | c, w := parseString(t, "output.DP-2.right-of = DP-1\noutput.DP-2.offset = 65536\noutput.DP-2.offset = -65536\n") | |||
| 706 | + | if len(w) != 1 || c.Outputs[0].Anchor.Offset != -65536 { | |||
| 707 | + | t.Fatalf("%v %+v", w, c.Outputs) | |||
| 708 | + | } | |||
| 709 | + | } | |||
| 710 | + | ||||
| 705 | func TestOutputModeLineKeepsAnchor(t *testing.T) { | 711 | func TestOutputModeLineKeepsAnchor(t *testing.T) { | ||
| 706 | c, w := parseString(t, "output.DP-2.right-of = DP-1\noutput.DP-2.offset = 50\noutput.DP-2 = 1920x1080\n") | 712 | c, w := parseString(t, "output.DP-2.right-of = DP-1\noutput.DP-2.offset = 50\noutput.DP-2 = 1920x1080\n") | ||
| 707 | if len(w) != 0 { | 713 | if len(w) != 0 { | ||