fix(validator): accept . , .. , and relative paths as COPY/ADD destination#132
Open
Fukuro192 wants to merge 5 commits intorcjsuen:masterfrom
Open
fix(validator): accept . , .. , and relative paths as COPY/ADD destination#132Fukuro192 wants to merge 5 commits intorcjsuen:masterfrom
Fukuro192 wants to merge 5 commits intorcjsuen:masterfrom
Conversation
…COPY/ADD On POSIX/Linux, . denotes the current directory, so COPY file1 file2 . and ADD file1 file2 . are valid. Relax the rule that the destination must end with / or \ to also accept . or ./ (with optional surrounding whitespace or quotes). - Update checkDestinationIsDirectory and checkJsonDestinationIsDirectory - Add tests for ADD and COPY with multiple sources and destination . or ./
…e COPY/ADD Follow-up to accepting . and ./: also accept parent directory (.. and ../) as valid directory destination for multi-source COPY/ADD on POSIX.
…ource COPY/ADD Use regex /^(\.\.?\/)*\.\.?\/?$/ so relative directory destinations like ../.. and ../../ are accepted, not only ., ./, .., ../. Keeps trailing-slash forms (./, ../) accepted. Add tests for trailing-slash regression and for deeper paths (../.., ../../).
… COPY/ADD Extend relative-dir regex to accept \ as path separator (e.g. ..\.., ..\) so Windows-style paths match the "end with / or \" rule. Add tests using
…gex helpers - Allow COPY/ADD destination to be a directory when path has segments and ends with / or \ (e.g. foo/bar/), in addition to . and .. - Add Validator regex constants and isValidDirectoryDestination() to replace duplicated logic in checkDestinationIsDirectory and checkJsonDestinationIsDirectory
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the false positive where the validator reported
"When using COPY/ADD with more than one source file, the destination must be a directory and end with a / or a \"for valid directory destinations.On POSIX,
.and..are the current and parent directories, soCOPY file1 file2 .,COPY file1 file2 .., and similar forms are valid. The Docker builder accepts them; perVALIDATOR.md, the validator should not error on Dockerfiles the builder can build.Changes
.,./,.., or../(after trimming and stripping optional surrounding quotes)./^(\.\.?[\/\\])*\.\.?[\/\\]?$/so any relative directory path is accepted (e.g.../..,../../,.././../../.), not only.and...foo/,foo/bar/,a/b/c/) and ends with/or\, using a path-segment regex plus an "ends with directory indicator" check.\as path separator (e.g...\..,..\,foo\bar\), consistent with the "end with/or\" rule.IS_RELATIVE_DIR,IS_PATH_WITH_DIR_NAMES,ENDS_WITH_DIR_INDICATOR) and a shared helperisValidDirectoryDestination(), used by bothcheckDestinationIsDirectoryandcheckJsonDestinationIsDirectory, removing duplicated logic..and./(shell and JSON) forADDandCOPY...and../../and../).../..,../../)...\..,..\) using#escape=so backslash is literal.The
invalidDestinationmessage text is unchanged; only the condition that triggers it was relaxed.Testing note
I've tested paths like
../..and../../on Docker (multi-sourceCOPY/ADDwith those destinations succeed).I haven't run paths using backslash (e.g.
..\..) 😅 ; the change is based on the documented rule that the destination may end with\and on the regex accepting the same relative-dir pattern with\as separator.Other Note
I'm not sure if too much for such a small fix -- I'd be happy to simplify to only the behavior change if so.