Make remaining bytes validation conditional on strict mode

[libark]

No description provided.

[coderabbitai]

Walkthrough

The pull request modifies error validation logic in src/any.rs and src/atom.rs to make UnderDecode checks conditional. Previously, decoding operations would always return an UnderDecode error when leftover bytes remained in the body after decoding. The changes introduce conditional compilation flags so that this validation only occurs when the "strict" feature is enabled or when tests are running. In standard non-strict builds, the remaining-body validation is skipped, allowing leftover bytes without producing errors. No public API signatures are modified.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to assess whether the description relates to the changeset.	Add a pull request description explaining the motivation, behavior changes, and impact of conditionalizing the remaining bytes validation.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: making remaining bytes validation conditional on strict mode, which is the primary modification across both modified files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/atom.rs`:
- Around line 66-69: Collapse the nested conditional checks into a single if
expression to satisfy clippy's collapsible_if lint: replace the outer
cfg!(feature = "strict") || cfg!(test) guard and the inner body.has_remaining()
check with one combined condition that tests both (e.g., if (cfg!(feature =
"strict") || cfg!(test)) && body.has_remaining()) and return
Err(Error::UnderDecode(T::KIND)) when true; apply the same change to the second
occurrence that currently returns Err(Error::UnderDecode(T::KIND)) when
body.has_remaining() under the cfg guard.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate the file and check its size
fd -t f "atom.rs" | head -20

Repository: kixelated/mp4-atom

Length of output: 91

---

🏁 Script executed:

# Read the relevant sections of src/atom.rs
cat -n src/atom.rs | sed -n '60,75p'

Repository: kixelated/mp4-atom

Length of output: 642

---

🏁 Script executed:

# Also check the second location
cat -n src/atom.rs | sed -n '140,155p'

Repository: kixelated/mp4-atom

Length of output: 678

---

🏁 Script executed:

# Check for other potential clippy issues in src/atom.rs
rg "if.*{" src/atom.rs -A 2 | head -50

Repository: kixelated/mp4-atom

Length of output: 135

---

🏁 Script executed:

# Search for nested if patterns more carefully
rg "if cfg!" src/atom.rs -A 2

Repository: kixelated/mp4-atom

Length of output: 358

---

🏁 Script executed:

# Also check the full context around both locations to ensure no other issues
cat -n src/atom.rs | sed -n '50,80p'

Repository: kixelated/mp4-atom

Length of output: 1157

---

Collapse the nested if blocks to unblock clippy.

Clippy flags these as collapsible_if, and with -D warnings this fails CI. Collapsing the condition resolves the lint at both sites.

🛠️ Proposed fix

-        if cfg!(feature = "strict") || cfg!(test) {
-            if body.has_remaining() {
-                return Err(Error::UnderDecode(T::KIND));
-            }
-        }
+        if (cfg!(feature = "strict") || cfg!(test)) && body.has_remaining() {
+            return Err(Error::UnderDecode(T::KIND));
+        }

-        if cfg!(feature = "strict") || cfg!(test) {
-            if body.has_remaining() {
-                return Err(Error::UnderDecode(T::KIND));
-            }
-        }
+        if (cfg!(feature = "strict") || cfg!(test)) && body.has_remaining() {
+            return Err(Error::UnderDecode(T::KIND));
+        }

Also applies to: 147-150

🧰 Tools

🪛 GitHub Actions: PR

[error] 66-66: Clippy: this if statement can be collapsed.

🪛 GitHub Check: build

[failure] 66-66:
this if statement can be collapsed

🤖 Prompt for AI Agents

In `@src/atom.rs` around lines 66 - 69, Collapse the nested conditional checks
into a single if expression to satisfy clippy's collapsible_if lint: replace the
outer cfg!(feature = "strict") || cfg!(test) guard and the inner
body.has_remaining() check with one combined condition that tests both (e.g., if
(cfg!(feature = "strict") || cfg!(test)) && body.has_remaining()) and return
Err(Error::UnderDecode(T::KIND)) when true; apply the same change to the second
occurrence that currently returns Err(Error::UnderDecode(T::KIND)) when
body.has_remaining() under the cfg guard.

[bradh]

Did this behaviour get mandated in ISOBMFF? I recall some discussion, but not the outcome.