fix(app): validate output transforms and guard cursor rotation
- commit
- 2ee8a716009fa1deec786141f6f705f71b631983
- author
- brice · · 2026-10-03 19:15 UTC
- parent
- 8d6e44d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +33 -19
M internal/app/cursor.go +2 -5
| @@ -81,13 +81,10 @@ func loadCursor(c ports.CursorChange, scale float64, t ports.BufferTransform, li | |||
| 81 | 81 | // rotateCursor returns img as an output with transform t holds it, hotspot | |
| 82 | 82 | // included. It never modifies img (theme images are cached). | |
| 83 | 83 | func rotateCursor(img ports.CursorImage, t ports.BufferTransform) ports.CursorImage { | |
| 84 | - | if t == 0 || img.W <= 0 || img.H <= 0 { | |
| 84 | + | if t == 0 || img.W <= 0 || img.H <= 0 || len(img.Pixels) < img.W*img.H*4 { | |
| 85 | 85 | return img | |
| 86 | 86 | } | |
| 87 | - | dw, dh := img.W, img.H | |
| 88 | - | if t.Rotated() { | |
| 89 | - | dw, dh = dh, dw | |
| 90 | - | } | |
| 87 | + | dw, dh := t.Size(img.W, img.H) | |
| 91 | 88 | out := make([]byte, dw*dh*4) | |
| 92 | 89 | for y := range img.H { | |
| 93 | 90 | for x := range img.W { | |
| @@ -81,13 +81,10 @@ func loadCursor(c ports.CursorChange, scale float64, t ports.BufferTransform, li | |||||
| 81 | // rotateCursor returns img as an output with transform t holds it, hotspot | 81 | // rotateCursor returns img as an output with transform t holds it, hotspot | ||
| 82 | // included. It never modifies img (theme images are cached). | 82 | // included. It never modifies img (theme images are cached). | ||
| 83 | func rotateCursor(img ports.CursorImage, t ports.BufferTransform) ports.CursorImage { | 83 | func rotateCursor(img ports.CursorImage, t ports.BufferTransform) ports.CursorImage { | ||
| 84 | - | if t == 0 || img.W <= 0 || img.H <= 0 { | 84 | + | if t == 0 || img.W <= 0 || img.H <= 0 || len(img.Pixels) < img.W*img.H*4 { |
| 85 | return img | 85 | return img | ||
| 86 | } | 86 | } | ||
| 87 | - | dw, dh := img.W, img.H | 87 | + | dw, dh := t.Size(img.W, img.H) |
| 88 | - | if t.Rotated() { | |||
| 89 | - | dw, dh = dh, dw | |||
| 90 | - | } | |||
| 91 | out := make([]byte, dw*dh*4) | 88 | out := make([]byte, dw*dh*4) | ||
| 92 | for y := range img.H { | 89 | for y := range img.H { | ||
| 93 | for x := range img.W { | 90 | for x := range img.W { | ||
M internal/app/cursor_test.go +5 -0
| @@ -159,6 +159,11 @@ func TestRotateCursor(t *testing.T) { | |||
| 159 | 159 | t.Fatalf("transform %d: hotspot (%d,%d) on pixel %d, want 5", tr, got.HotX, got.HotY, v) | |
| 160 | 160 | } | |
| 161 | 161 | } | |
| 162 | + | // Pixels shorter than W*H*4 are returned as they are. | |
| 163 | + | short := ports.CursorImage{W: 3, H: 2, Pixels: px[:8]} | |
| 164 | + | if got := rotateCursor(short, 1); got.W != 3 || got.H != 2 || len(got.Pixels) != 8 { | |
| 165 | + | t.Fatalf("short pixels: %+v", got) | |
| 166 | + | } | |
| 162 | 167 | // 90 degrees: the top left pixel goes to the bottom left. | |
| 163 | 168 | if got := rotateCursor(ports.CursorImage{W: 3, H: 2, Pixels: px}, 1); got.Pixels[(2*2+0)*4] != 0 { | |
| 164 | 169 | t.Fatalf("90: top-left pixel at %v", got.Pixels) | |
| @@ -159,6 +159,11 @@ func TestRotateCursor(t *testing.T) { | |||||
| 159 | t.Fatalf("transform %d: hotspot (%d,%d) on pixel %d, want 5", tr, got.HotX, got.HotY, v) | 159 | t.Fatalf("transform %d: hotspot (%d,%d) on pixel %d, want 5", tr, got.HotX, got.HotY, v) | ||
| 160 | } | 160 | } | ||
| 161 | } | 161 | } | ||
| 162 | + | // Pixels shorter than W*H*4 are returned as they are. | |||
| 163 | + | short := ports.CursorImage{W: 3, H: 2, Pixels: px[:8]} | |||
| 164 | + | if got := rotateCursor(short, 1); got.W != 3 || got.H != 2 || len(got.Pixels) != 8 { | |||
| 165 | + | t.Fatalf("short pixels: %+v", got) | |||
| 166 | + | } | |||
| 162 | // 90 degrees: the top left pixel goes to the bottom left. | 167 | // 90 degrees: the top left pixel goes to the bottom left. | ||
| 163 | if got := rotateCursor(ports.CursorImage{W: 3, H: 2, Pixels: px}, 1); got.Pixels[(2*2+0)*4] != 0 { | 168 | if got := rotateCursor(ports.CursorImage{W: 3, H: 2, Pixels: px}, 1); got.Pixels[(2*2+0)*4] != 0 { | ||
| 164 | t.Fatalf("90: top-left pixel at %v", got.Pixels) | 169 | t.Fatalf("90: top-left pixel at %v", got.Pixels) | ||
M internal/app/outputmanagement.go +8 -1
| @@ -115,6 +115,9 @@ func (o *outputOverrides) validate(req ports.OutputApply) error { | |||
| 115 | 115 | if change.Scale < 0 || change.Scale > 4 || (change.Scale > 0 && change.Scale < 1) || math.IsNaN(change.Scale) || math.IsInf(change.Scale, 0) { | |
| 116 | 116 | return fmt.Errorf("invalid scale on %s", change.Name) | |
| 117 | 117 | } | |
| 118 | + | if change.Transform != nil && *change.Transform > 7 { | |
| 119 | + | return fmt.Errorf("invalid transform on %s", change.Name) | |
| 120 | + | } | |
| 118 | 121 | if o.headless && (change.Enabled != current.Enabled || change.Mode != nil && (current.Current == nil || change.Mode.Width != current.Current.Width || change.Mode.Height != current.Current.Height || change.Mode.RefreshMilli != current.Current.RefreshMilli)) { | |
| 119 | 122 | return fmt.Errorf("headless mode and enabled state are fixed") | |
| 120 | 123 | } | |
| @@ -283,7 +286,11 @@ func (a *outputApply) finished(err error) { | |||
| 283 | 286 | } | |
| 284 | 287 | a.req = nil | |
| 285 | 288 | for _, h := range req.Heads { | |
| 286 | - | a.log.Info().Str("output", h.Name).Bool("enabled", h.Enabled).Float64("scale", h.Scale).Msg("output configuration applied") | |
| 289 | + | e := a.log.Info().Str("output", h.Name).Bool("enabled", h.Enabled).Float64("scale", h.Scale) | |
| 290 | + | if h.Transform != nil { | |
| 291 | + | e = e.Uint8("transform", uint8(*h.Transform)) | |
| 292 | + | } | |
| 293 | + | e.Msg("output configuration applied") | |
| 287 | 294 | } | |
| 288 | 295 | a.reply(req.ID, nil) | |
| 289 | 296 | } | |
| @@ -115,6 +115,9 @@ func (o *outputOverrides) validate(req ports.OutputApply) error { | |||||
| 115 | if change.Scale < 0 || change.Scale > 4 || (change.Scale > 0 && change.Scale < 1) || math.IsNaN(change.Scale) || math.IsInf(change.Scale, 0) { | 115 | if change.Scale < 0 || change.Scale > 4 || (change.Scale > 0 && change.Scale < 1) || math.IsNaN(change.Scale) || math.IsInf(change.Scale, 0) { | ||
| 116 | return fmt.Errorf("invalid scale on %s", change.Name) | 116 | return fmt.Errorf("invalid scale on %s", change.Name) | ||
| 117 | } | 117 | } | ||
| 118 | + | if change.Transform != nil && *change.Transform > 7 { | |||
| 119 | + | return fmt.Errorf("invalid transform on %s", change.Name) | |||
| 120 | + | } | |||
| 118 | if o.headless && (change.Enabled != current.Enabled || change.Mode != nil && (current.Current == nil || change.Mode.Width != current.Current.Width || change.Mode.Height != current.Current.Height || change.Mode.RefreshMilli != current.Current.RefreshMilli)) { | 121 | if o.headless && (change.Enabled != current.Enabled || change.Mode != nil && (current.Current == nil || change.Mode.Width != current.Current.Width || change.Mode.Height != current.Current.Height || change.Mode.RefreshMilli != current.Current.RefreshMilli)) { | ||
| 119 | return fmt.Errorf("headless mode and enabled state are fixed") | 122 | return fmt.Errorf("headless mode and enabled state are fixed") | ||
| 120 | } | 123 | } | ||
| @@ -283,7 +286,11 @@ func (a *outputApply) finished(err error) { | |||||
| 283 | } | 286 | } | ||
| 284 | a.req = nil | 287 | a.req = nil | ||
| 285 | for _, h := range req.Heads { | 288 | for _, h := range req.Heads { | ||
| 286 | - | a.log.Info().Str("output", h.Name).Bool("enabled", h.Enabled).Float64("scale", h.Scale).Msg("output configuration applied") | 289 | + | e := a.log.Info().Str("output", h.Name).Bool("enabled", h.Enabled).Float64("scale", h.Scale) |
| 290 | + | if h.Transform != nil { | |||
| 291 | + | e = e.Uint8("transform", uint8(*h.Transform)) | |||
| 292 | + | } | |||
| 293 | + | e.Msg("output configuration applied") | |||
| 287 | } | 294 | } | ||
| 288 | a.reply(req.ID, nil) | 295 | a.reply(req.ID, nil) | ||
| 289 | } | 296 | } | ||
M internal/app/outputmanagement_test.go +18 -13
| @@ -286,19 +286,6 @@ func TestOutputApplyAnsweredAtOnce(t *testing.T) { | |||
| 286 | 286 | ||
| 287 | 287 | // A reload that only changes output scales (a saved scale bind) keeps the | |
| 288 | 288 | // runtime overrides; any other change drops them. | |
| 289 | - | func TestScaleChangesTransform(t *testing.T) { | |
| 290 | - | old := ports.Config{Outputs: []ports.OutputConfig{{Name: "DP-1", Scale: 2}}} | |
| 291 | - | cur := ports.Config{Outputs: []ports.OutputConfig{{Name: "DP-1", Scale: 2, Transform: 1}}} | |
| 292 | - | if _, only := scaleChanges(old, cur); only { | |
| 293 | - | t.Fatal("a transform change must take the full reload path") | |
| 294 | - | } | |
| 295 | - | cur.Outputs[0].Transform = 0 | |
| 296 | - | cur.Outputs[0].Scale = 1.5 | |
| 297 | - | if _, only := scaleChanges(old, cur); !only { | |
| 298 | - | t.Fatal("a scale-only change stays a scale change") | |
| 299 | - | } | |
| 300 | - | } | |
| 301 | - | ||
| 302 | 289 | func TestReloadScaleOnlyKeepsOverrides(t *testing.T) { | |
| 303 | 290 | mode := ports.OutputMode{Width: 1920, Height: 1080, RefreshMilli: 60000} | |
| 304 | 291 | file := ports.Config{Outputs: []ports.OutputConfig{{Name: "DP-1", Scale: 2}}} | |
| @@ -320,6 +307,20 @@ func TestReloadScaleOnlyKeepsOverrides(t *testing.T) { | |||
| 320 | 307 | } | |
| 321 | 308 | } | |
| 322 | 309 | ||
| 310 | + | // A transform change takes the full reload path; a scale change alone does not. | |
| 311 | + | func TestScaleChangesTransform(t *testing.T) { | |
| 312 | + | old := ports.Config{Outputs: []ports.OutputConfig{{Name: "DP-1", Scale: 2}}} | |
| 313 | + | cur := ports.Config{Outputs: []ports.OutputConfig{{Name: "DP-1", Scale: 2, Transform: 1}}} | |
| 314 | + | if _, only := scaleChanges(old, cur); only { | |
| 315 | + | t.Fatal("a transform change must take the full reload path") | |
| 316 | + | } | |
| 317 | + | cur.Outputs[0].Transform = 0 | |
| 318 | + | cur.Outputs[0].Scale = 1.5 | |
| 319 | + | if _, only := scaleChanges(old, cur); !only { | |
| 320 | + | t.Fatal("a scale-only change stays a scale change") | |
| 321 | + | } | |
| 322 | + | } | |
| 323 | + | ||
| 323 | 324 | // A runtime transform lands in the effective configuration and survives a | |
| 324 | 325 | // reload that only changes file scales. | |
| 325 | 326 | func TestOutputApplyTransform(t *testing.T) { | |
| @@ -345,6 +346,10 @@ func TestOutputApplyTransform(t *testing.T) { | |||
| 345 | 346 | if err != nil || cfg.Outputs[0].Transform != 3 { | |
| 346 | 347 | t.Fatalf("unset transform changed: %+v, %v", cfg.Outputs, err) | |
| 347 | 348 | } | |
| 349 | + | bad := ports.BufferTransform(8) | |
| 350 | + | if _, err := state.apply(ports.OutputApply{Heads: []ports.HeadChange{{Name: "DP-1", Enabled: true, Transform: &bad}}}); err == nil { | |
| 351 | + | t.Fatal("transform 8 accepted") | |
| 352 | + | } | |
| 348 | 353 | } | |
| 349 | 354 | ||
| 350 | 355 | // A runtime position never replaces a file relation: it only takes | |
| @@ -286,19 +286,6 @@ func TestOutputApplyAnsweredAtOnce(t *testing.T) { | |||||
| 286 | 286 | ||||
| 287 | // A reload that only changes output scales (a saved scale bind) keeps the | 287 | // A reload that only changes output scales (a saved scale bind) keeps the | ||
| 288 | // runtime overrides; any other change drops them. | 288 | // runtime overrides; any other change drops them. | ||
| 289 | - | func TestScaleChangesTransform(t *testing.T) { | |||
| 290 | - | old := ports.Config{Outputs: []ports.OutputConfig{{Name: "DP-1", Scale: 2}}} | |||
| 291 | - | cur := ports.Config{Outputs: []ports.OutputConfig{{Name: "DP-1", Scale: 2, Transform: 1}}} | |||
| 292 | - | if _, only := scaleChanges(old, cur); only { | |||
| 293 | - | t.Fatal("a transform change must take the full reload path") | |||
| 294 | - | } | |||
| 295 | - | cur.Outputs[0].Transform = 0 | |||
| 296 | - | cur.Outputs[0].Scale = 1.5 | |||
| 297 | - | if _, only := scaleChanges(old, cur); !only { | |||
| 298 | - | t.Fatal("a scale-only change stays a scale change") | |||
| 299 | - | } | |||
| 300 | - | } | |||
| 301 | - | ||||
| 302 | func TestReloadScaleOnlyKeepsOverrides(t *testing.T) { | 289 | func TestReloadScaleOnlyKeepsOverrides(t *testing.T) { | ||
| 303 | mode := ports.OutputMode{Width: 1920, Height: 1080, RefreshMilli: 60000} | 290 | mode := ports.OutputMode{Width: 1920, Height: 1080, RefreshMilli: 60000} | ||
| 304 | file := ports.Config{Outputs: []ports.OutputConfig{{Name: "DP-1", Scale: 2}}} | 291 | file := ports.Config{Outputs: []ports.OutputConfig{{Name: "DP-1", Scale: 2}}} | ||
| @@ -320,6 +307,20 @@ func TestReloadScaleOnlyKeepsOverrides(t *testing.T) { | |||||
| 320 | } | 307 | } | ||
| 321 | } | 308 | } | ||
| 322 | 309 | ||||
| 310 | + | // A transform change takes the full reload path; a scale change alone does not. | |||
| 311 | + | func TestScaleChangesTransform(t *testing.T) { | |||
| 312 | + | old := ports.Config{Outputs: []ports.OutputConfig{{Name: "DP-1", Scale: 2}}} | |||
| 313 | + | cur := ports.Config{Outputs: []ports.OutputConfig{{Name: "DP-1", Scale: 2, Transform: 1}}} | |||
| 314 | + | if _, only := scaleChanges(old, cur); only { | |||
| 315 | + | t.Fatal("a transform change must take the full reload path") | |||
| 316 | + | } | |||
| 317 | + | cur.Outputs[0].Transform = 0 | |||
| 318 | + | cur.Outputs[0].Scale = 1.5 | |||
| 319 | + | if _, only := scaleChanges(old, cur); !only { | |||
| 320 | + | t.Fatal("a scale-only change stays a scale change") | |||
| 321 | + | } | |||
| 322 | + | } | |||
| 323 | + | ||||
| 323 | // A runtime transform lands in the effective configuration and survives a | 324 | // A runtime transform lands in the effective configuration and survives a | ||
| 324 | // reload that only changes file scales. | 325 | // reload that only changes file scales. | ||
| 325 | func TestOutputApplyTransform(t *testing.T) { | 326 | func TestOutputApplyTransform(t *testing.T) { | ||
| @@ -345,6 +346,10 @@ func TestOutputApplyTransform(t *testing.T) { | |||||
| 345 | if err != nil || cfg.Outputs[0].Transform != 3 { | 346 | if err != nil || cfg.Outputs[0].Transform != 3 { | ||
| 346 | t.Fatalf("unset transform changed: %+v, %v", cfg.Outputs, err) | 347 | t.Fatalf("unset transform changed: %+v, %v", cfg.Outputs, err) | ||
| 347 | } | 348 | } | ||
| 349 | + | bad := ports.BufferTransform(8) | |||
| 350 | + | if _, err := state.apply(ports.OutputApply{Heads: []ports.HeadChange{{Name: "DP-1", Enabled: true, Transform: &bad}}}); err == nil { | |||
| 351 | + | t.Fatal("transform 8 accepted") | |||
| 352 | + | } | |||
| 348 | } | 353 | } | ||
| 349 | 354 | ||||
| 350 | // A runtime position never replaces a file relation: it only takes | 355 | // A runtime position never replaces a file relation: it only takes | ||