fix: verify_decode accepts a value that silently skips the decode check
The decode check after filing runs on the sampled files, on every file, or not at all. An unrecognised verify_decode was stored verbatim and then matched none of those modes, so a typo quietly turned the only end-to-end check of the audio that gets written into a no-op. It is restricted to all, sample or none. quality is range-checked at the same time: libvorbis accepts 1-10, and anything outside that reached ffmpeg and failed the whole album with an encoder error instead of a configuration one. Closes 175074d
diff --git a/internal/config/config.go b/internal/config/config.go
@@ -143,6 +143,9 @@ func (c *Config) set(key, value string) error {
if err != nil {
return fmt.Errorf("quality: %q is not a number", value)
}
+ if n < 1 || n > 10 {
+ return fmt.Errorf("quality: %d is outside the range 1-10 that libvorbis accepts", n)
+ }
c.Quality = n
case "disc_prefix":
v := strings.ToLower(strings.TrimSpace(value))
@@ -179,7 +182,17 @@ func (c *Config) set(key, value string) error {
}
c.KeepStaging = b
case "verify_decode":
- c.VerifyDecode = value
+ // The decode check after filing runs on the sampled files, on every
+ // file, or not at all. An unrecognised value used to be stored as it was
+ // and then matched none of those, so a typo quietly turned the only
+ // end-to-end check of the written audio into a no-op.
+ v := strings.ToLower(strings.TrimSpace(value))
+ switch v {
+ case "all", "sample", "none":
+ c.VerifyDecode = v
+ default:
+ return fmt.Errorf("verify_decode: %q is not one of all, sample or none", value)
+ }
case "offline":
b, err := strconv.ParseBool(value)
if err != nil {
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
@@ -31,3 +31,34 @@ func TestDiscPrefixRejectsUnknownValues(t *testing.T) {
}
}
}
+
+// An unrecognised verify_decode used to be stored verbatim and then matched none
+// of the modes the verifier knows, so a typo quietly turned the only end-to-end
+// check of the audio that gets written into a no-op.
+func TestVerifyDecodeRejectsUnknownValues(t *testing.T) {
+ for _, good := range []string{"all", "sample", "none", "SAMPLE", " all "} {
+ if _, err := Load(write(t, "verify_decode = \""+good+"\"")); err != nil {
+ t.Errorf("verify_decode = %q should be accepted: %v", good, err)
+ }
+ }
+ for _, bad := range []string{"", "sample2", "yes", "true"} {
+ if _, err := Load(write(t, "verify_decode = \""+bad+"\"")); err == nil {
+ t.Errorf("verify_decode = %q should be rejected", bad)
+ }
+ }
+}
+
+// libvorbis takes 1-10, and a value outside that used to reach ffmpeg, which
+// failed the whole album with an encoder error rather than a configuration one.
+func TestQualityRangeIsChecked(t *testing.T) {
+ for _, good := range []string{"1", "9", "10"} {
+ if _, err := Load(write(t, "quality = "+good)); err != nil {
+ t.Errorf("quality = %s should be accepted: %v", good, err)
+ }
+ }
+ for _, bad := range []string{"0", "11", "-3"} {
+ if _, err := Load(write(t, "quality = "+bad)); err == nil {
+ t.Errorf("quality = %s should be rejected", bad)
+ }
+ }
+}