| Message ID | 20241203210652.GA1413195@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8cb4c6e62f28ac9d9a04daf794ff6dbddf55e416 |
| Headers | show |
| Series | [v2] fast-import: disallow more path components | expand |
On Tue, Dec 3, 2024 at 1:07 PM Jeff King <peff@peff.net> wrote: > > On Tue, Dec 03, 2024 at 12:01:51AM -0800, Elijah Newren wrote: > > > > On Sat, Nov 30, 2024 at 01:09:29AM +0000, Elijah Newren via GitGitGadget wrote: > > > > > > > Changes since v1: > > > > > > > > * Moved the check to a higher level, as suggested by Peff. > > > > > > Thanks, the code change looks good. Is it worth tweaking one of the > > > tests to do "R innocent-path .git/evil"? Otherwise I don't think there's > > > any coverage of the file_change_cr() call at all. > > > > I would say yes, but since this patch too has made it to next and is > > marked for master, I'm kinda tempted to just leave it as-is... > > Is is tempting. :) I wrote this up, though, which can just go on top (of > en/fast-import-verify-path). Thanks! > -Peff > > -- >8 -- > Subject: [PATCH] t9300: test verification of renamed paths > > Commit da91a90c2f (fast-import: disallow more path components, > 2024-11-30) added two separate verify_path() calls (one for > added/modified files, and one for renames/copies). But our tests only > exercise the first one. Let's protect ourselves against regressions by > tweaking one of the tests to rename into the bad path. There are > adjacent tests that will stay as additions, so now both calls are > covered. > > Signed-off-by: Jeff King <peff@peff.net> > --- > t/t9300-fast-import.sh | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh > index e2b1db6bc2..fd01a2353c 100755 > --- a/t/t9300-fast-import.sh > +++ b/t/t9300-fast-import.sh > @@ -553,9 +553,16 @@ test_expect_success 'B: fail on invalid file path of .' ' > commit refs/heads/badpath > committer Name <email> $GIT_COMMITTER_DATE > data <<COMMIT > - Commit Message > + Good path > + COMMIT > + M 100644 :1 ok-path > + > + commit refs/heads/badpath > + committer Name <email> $GIT_COMMITTER_DATE > + data <<COMMIT > + Bad path > COMMIT > - M 100644 :1 ./invalid-path > + R ok-path ./invalid-path > INPUT_END > > test_when_finished "git update-ref -d refs/heads/badpath" && > -- > 2.47.1.707.g92f6f18526 Change looks good to me.
Jeff King <peff@peff.net> writes: >> I would say yes, but since this patch too has made it to next and is >> marked for master, I'm kinda tempted to just leave it as-is... > > Is is tempting. :) I wrote this up, though, which can just go on top (of > en/fast-import-verify-path). Thanks, queued. > -- >8 -- > Subject: [PATCH] t9300: test verification of renamed paths > > Commit da91a90c2f (fast-import: disallow more path components, > 2024-11-30) added two separate verify_path() calls (one for > added/modified files, and one for renames/copies). But our tests only > exercise the first one. Let's protect ourselves against regressions by > tweaking one of the tests to rename into the bad path. There are > adjacent tests that will stay as additions, so now both calls are > covered. > > Signed-off-by: Jeff King <peff@peff.net> > --- > t/t9300-fast-import.sh | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh > index e2b1db6bc2..fd01a2353c 100755 > --- a/t/t9300-fast-import.sh > +++ b/t/t9300-fast-import.sh > @@ -553,9 +553,16 @@ test_expect_success 'B: fail on invalid file path of .' ' > commit refs/heads/badpath > committer Name <email> $GIT_COMMITTER_DATE > data <<COMMIT > - Commit Message > + Good path > + COMMIT > + M 100644 :1 ok-path > + > + commit refs/heads/badpath > + committer Name <email> $GIT_COMMITTER_DATE > + data <<COMMIT > + Bad path > COMMIT > - M 100644 :1 ./invalid-path > + R ok-path ./invalid-path > INPUT_END > > test_when_finished "git update-ref -d refs/heads/badpath" &&
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index e2b1db6bc2..fd01a2353c 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -553,9 +553,16 @@ test_expect_success 'B: fail on invalid file path of .' ' commit refs/heads/badpath committer Name <email> $GIT_COMMITTER_DATE data <<COMMIT - Commit Message + Good path + COMMIT + M 100644 :1 ok-path + + commit refs/heads/badpath + committer Name <email> $GIT_COMMITTER_DATE + data <<COMMIT + Bad path COMMIT - M 100644 :1 ./invalid-path + R ok-path ./invalid-path INPUT_END test_when_finished "git update-ref -d refs/heads/badpath" &&