fix(icc): validate the CLI arguments and the IPC parameters of the ICC commands

Sweep of the two surfaces the review has not looked at yet, so the failures are
found here instead of in the next round.

- `dms icc apply` copied the profile into the config directory before asking the
  daemon, and only the *parse* was checked locally. A profile the daemon refuses
  (no `vcgt` table, non-RGB color space) was therefore left in the profile
  directory for `dms icc list` to keep offering, and an IPC failure left the copy
  behind as well. The command now checks the same rules the daemon enforces
  before copying anything, and removes the copy it just made when the request
  fails (a file that was already there is left alone).
- `dms icc list` marks a profile without a `vcgt` table, so the rule that makes
  `apply` refuse it is visible where the profiles are listed.
- `ApplyICC`/`RemoveICC`/`SetOutputTemp` reach the manager over IPC with whatever
  the caller sent: an empty (or whitespace-padded) output name used to be
  persisted as an entry in `wayland.json` that no `wl_output` can ever match, and
  a relative profile path was stored to be resolved again on the next start, from
  a different working directory. Both are rejected now, before anything is
  written.

Verified with the built CLI against the previous binary: with a profile that has
no vcgt table, `dms icc apply` now exits before touching the config directory
(`residual=[]`) where the previous one copied the file and went on to the IPC
(`residual=[nogamma.icc]`); with a valid profile and no daemon reachable, the
previous one leaves `vcgt.icc` behind and this one does not. Covered by
TestManager_RejectsInvalidICCArguments.
This commit is contained in:
KIDult2226
2026-09-14 22:07:13 +08:00
parent 9da245dcc9
commit cfe93cc0d4
3 changed files with 105 additions and 4 deletions
+36 -4
View File
@@ -189,7 +189,14 @@ func runICCList(cmd *cobra.Command, args []string) {
fmt.Printf(" %-40s (parse error: %v)\n", filepath.Base(path), err)
continue
}
fmt.Printf(" %-40s %s\n", filepath.Base(path), profile.Description)
// A profile without a vcgt table has no video card gamma ramp, so the
// daemon refuses to apply it: say so here instead of letting the user
// find out from a rejected `dms icc apply`.
note := ""
if !profile.HasVCGT {
note = " (no vcgt table: cannot be applied)"
}
fmt.Printf(" %-40s %s%s\n", filepath.Base(path), profile.Description, note)
}
}
}
@@ -271,11 +278,20 @@ func runICCApply(cmd *cobra.Command, args []string) {
filePath = abs
}
// Validate file exists and is a valid ICC profile
// Validate file exists and is a valid ICC profile. The rules the daemon
// enforces are checked here too: a profile it would refuse must not be copied
// into the profile directory first, or `dms icc list` keeps offering a file
// that can never be applied.
profile, err := icc.ParseFile(filePath)
if err != nil {
log.Fatalf("Invalid ICC profile: %v", err)
}
if profile.ColorSpace != "RGB" {
log.Fatalf("Unsupported color space %q, only RGB profiles can be applied", profile.ColorSpace)
}
if !profile.HasVCGT {
log.Fatalf("%s has no vcgt table, so it has no gamma ramp to apply", filepath.Base(filePath))
}
// Copy the ICC file to config dir for persistence
configDir := getICCConfigDir()
@@ -284,7 +300,11 @@ func runICCApply(cmd *cobra.Command, args []string) {
}
destPath := filepath.Join(configDir, filepath.Base(filePath))
copied := false
if filepath.Clean(filePath) != filepath.Clean(destPath) {
if _, err := os.Stat(destPath); err != nil {
copied = true
}
data, err := os.ReadFile(filePath)
if err != nil {
log.Fatalf("Failed to read ICC file: %v", err)
@@ -302,19 +322,31 @@ func runICCApply(cmd *cobra.Command, args []string) {
"path": destPath,
},
}
resp, err := sendServerRequest(req)
if err != nil {
dropCopiedProfile(destPath, copied)
log.Fatalf("Failed to apply ICC profile: %v", err)
}
if resp.Error != "" {
dropCopiedProfile(destPath, copied)
log.Fatalf("Server error: %s", resp.Error)
}
fmt.Printf("Applied ICC profile '%s' to output '%s'\n", profile.Description, outputName)
}
// dropCopiedProfile removes the copy this command just made, so a rejected apply
// does not leave a profile in the config directory that was never applied (a
// file that was already there is left alone).
func dropCopiedProfile(path string, copied bool) {
if !copied {
return
}
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
log.Errorf("Failed to remove %s: %v", path, err)
}
}
func runICCRemove(cmd *cobra.Command, args []string) {
outputName := args[0]
+42
View File
@@ -8,7 +8,9 @@ import (
"io"
"maps"
"os"
"path/filepath"
"slices"
"strings"
"syscall"
"time"
@@ -1551,10 +1553,43 @@ func (m *Manager) applyConfiguredICCForOutput(out *outputState, outputName strin
return changed
}
// validateOutputName rejects names no wl_output can report, so a typo or an
// empty argument cannot persist an entry in wayland.json that nothing will ever
// match.
func validateOutputName(name string) error {
switch {
case name == "":
return errors.New("icc: output name is required")
case strings.TrimSpace(name) != name:
return fmt.Errorf("icc: output name %q has surrounding whitespace", name)
}
return nil
}
// validateICCProfilePath rejects paths that cannot be resolved later: the path
// is persisted and read again on the next start, when the daemon no longer has
// the working directory the caller had.
func validateICCProfilePath(path string) error {
switch {
case path == "":
return errors.New("icc: profile path is required")
case !filepath.IsAbs(path):
return fmt.Errorf("icc: profile path %q must be absolute", path)
}
return nil
}
// ApplyICC loads and applies an ICC profile to a specific output.
// outputName is the wl_output name (e.g., "DP-1").
// iccPath is the path to the .icm/.icc profile file.
func (m *Manager) ApplyICC(outputName, iccPath string) error {
if err := validateOutputName(outputName); err != nil {
return err
}
if err := validateICCProfilePath(iccPath); err != nil {
return err
}
// 1. Parse and validate the ICC profile
profile, err := icc.ParseFile(iccPath)
if err != nil {
@@ -1624,6 +1659,10 @@ func (m *Manager) ApplyICC(outputName, iccPath string) error {
// RemoveICC removes the ICC profile from a specific output, reverting to temperature-based gamma.
func (m *Manager) RemoveICC(outputName string) error {
if err := validateOutputName(outputName); err != nil {
return err
}
m.post(func() {
defer m.updateStateFromSchedule()
@@ -1745,6 +1784,9 @@ func (m *Manager) ListOutputs() []string {
// SetOutputTemp sets a per-output color temperature (1000K-10000K).
// A value of 0 resets to the global default.
func (m *Manager) SetOutputTemp(outputName string, temp int) error {
if err := validateOutputName(outputName); err != nil {
return err
}
if temp != 0 && (temp < 1000 || temp > 10000) {
return fmt.Errorf("temperature %d out of range (1000-10000)", temp)
}
@@ -915,3 +915,30 @@ func TestManager_SetOutputTempWithoutControlsPublishesValue(t *testing.T) {
}
assert.Equal(t, 7000, saved.OutputTemps["DP-1"], "and persisted to the config file")
}
// Arguments that no wl_output or file can satisfy are rejected before anything is
// written: an empty name would persist an entry in wayland.json that nothing can
// match, and a relative profile path cannot be resolved by the next start, when
// the daemon no longer has the caller's working directory.
func TestManager_RejectsInvalidICCArguments(t *testing.T) {
m := &Manager{config: DefaultConfig()}
t.Run("empty output name", func(t *testing.T) {
assert.Error(t, m.ApplyICC("", "/tmp/profile.icc"))
assert.Error(t, m.RemoveICC(""))
assert.Error(t, m.SetOutputTemp("", 7000))
assert.Empty(t, m.config.ICCProfiles, "nothing may be stored for a name that cannot exist")
assert.Empty(t, m.config.OutputTemps)
})
t.Run("output name with whitespace", func(t *testing.T) {
assert.Error(t, m.SetOutputTemp(" DP-1", 7000))
assert.Empty(t, m.config.OutputTemps)
})
t.Run("relative profile path", func(t *testing.T) {
assert.Error(t, m.ApplyICC("DP-1", "profile.icc"))
assert.Error(t, m.ApplyICC("DP-1", ""))
assert.Empty(t, m.config.ICCProfiles)
})
}