Skip to content

parsevolume: fix escape guard and option normalization - #7035

Open
vtushar06 wants to merge 2 commits into
podman-container-tools:mainfrom
vtushar06:parsevolume-fixes
Open

parsevolume: fix escape guard and option normalization#7035
vtushar06 wants to merge 2 commits into
podman-container-tools:mainfrom
vtushar06:parsevolume-fixes

Conversation

@vtushar06

@vtushar06 vtushar06 commented Aug 14, 2026

Copy link
Copy Markdown

I was reading the volume parser and noticed the escape guard in SplitStringWithColonEscape checks idx-1 > 0, which excludes index 1, so a volume starting with an escaped colon gets split on it anyway:

\:foo:/dst  ->  ["\", "foo", "/dst"]   (expected [":foo", "/dst"])

Mid-string escapes are unaffected. I don't think this is reachable from the CLI today since the host path has to be absolute, so I am pitching it as correctness plus coverage, not a live bug.

While writing tests for it I also noticed Volume() discards the normalized slice ValidateVolumeOpts returns, so the cached/delegated options that function is documented to silently drop still end up in mount.Options, and with no options at all mount.Options comes back as [""] instead of empty.

so the package had no tests, so both commits bring their own here
cc @lsm5

The escape guard tests idx-1 > 0, which excludes index 1, so a volume
starting with an escaped colon is split on that colon anyway:
\:foo:/dst gives ["\", "foo", "/dst"] instead of [":foo", "/dst"].
Not reachable from the CLI today since the host path has to be
absolute, so this is correctness rather than a live bug.

The package had no tests at all, so add the first ones alongside.

Signed-off-by: Tushar Verma <tusharmyself06@gmail.com>
Volume() throws away the normalized slice ValidateVolumeOpts returns,
so the cached and delegated options that function is documented to
drop silently still reach mount.Options. With no options at all,
splitting an empty string left mount.Options as [""] rather than
empty.

Signed-off-by: Tushar Verma <tusharmyself06@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant