Repository navigation
Split mount directives on the first equals sign - #2016
devops-thiago wants to merge 1 commit into
Conversation
49dfdd8 to
1c9c86b
Compare
1c9c86b to
8b45175
Compare
8b45175 to
f48ceec
Compare
f48ceec to
62e9214
Compare
62e9214 to
673eb30
Compare
673eb30 to
3262fae
Compare
Parser.mount split each comma-separated directive with maxSplits: 2, which yields three components when the value itself contains an equals sign. The parser then requires exactly two components, so any mount whose source or destination path contains "=" failed with "invalid directive format missing value". Equals signs are legal in POSIX path names. Splitting on the first equals sign keeps the remainder as the value. Fixes apple#2012
3262fae to
e255669
Compare
|
Asking a few more people for review. I previously requested reviews from @egernst and @jglogan on #2019, but haven’t heard back, and CONTRIBUTING suggests trying someone else after a few weeks. The only review so far is katiewasnothere’s on #2329 — thanks. I replied there. I have 11 small PRs open. CI has not run on any of them because the workflows are still waiting for approval ( I tried to match each PR to areas you’ve worked on, but anyone is welcome to review any of them:
All commits are signed and the branches are rebased on For each bug, I reproduced the issue on the 1.5.0 release and included the output in the PR. I also tested each fix from a build of its branch. Those fixes started as fixes in the engine used by SiliconShip, a Mac app I build: https://apps.apple.com/app/id6797506428. I’m happy to split, shrink, or close any of these if that would make review easier. |
Code Coverage
|
|
@dkovba thank you for approving the workflows. CI passes on this PR and it still merges cleanly into main. Is it good to merge as it is, or is there anything else you need from me? |
|
@katiewasnothere this has been open since Jul 25 without a review. CI passes and it merges cleanly into main. Could you take a look when you have time, or tell me who else I should ask? |
Type of Change
Motivation and Context
Fixes #2012.
Parser.mountsplits each comma-separated directive withmaxSplits: 2. In Swift that parameter counts splits rather than resulting elements, so a value that itself contains an equals sign produces three components:The parser then requires exactly two components and throws
invalid directive format missing value. Equals signs are legal in POSIX path names, so a bind mount whose source or destination contains one cannot be used today.Splitting on the first equals sign keeps the remainder as the value. Directives that carry no value, such as
readonly, still produce a single component and are unaffected.#1978 and #1999 fix the same defect in
Parser.labels. Neither changesParser.mount.Testing
testMountBindSourceContainingEqualscreates a directory whose name contains an equals sign and mounts it by absolute path. Against the current code the test fails withinvalid directive format missing value; with this change it passes. The existing mount and publish-port tests continue to pass.I also ran a debug build of this branch against a 1.5.0 daemon with the command from the issue. The 1.5.0 release and a build of main fail with
invalid directive format missing value. This branch starts the container, andmountinside it showsvirtiofs on /mnt type virtiofs (rw,relatime).