Add validation to check for file path overlaps #420
Merged
Conversation
This file contains 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
daviddavis
pushed a commit
to daviddavis/pulp_file
that referenced
this pull request
Nov 26, 2019
ref #5559 Required PR: pulp/pulpcore#420
bmbouter
reviewed
Nov 26, 2019
daviddavis
pushed a commit
to daviddavis/pulp_file
that referenced
this pull request
Nov 26, 2019
ref #5559 Required PR: pulp/pulpcore#420
daviddavis
force-pushed
the
issue5559
branch
from
67fb077
to
46e6545
Compare
daviddavis
force-pushed
the
issue5559
branch
from
46e6545
to
aa6076d
Compare
daviddavis
pushed a commit
to daviddavis/pulp_file
that referenced
this pull request
Nov 26, 2019
ref #5559 Required PR: pulp/pulpcore#420
daviddavis
force-pushed
the
issue5559
branch
from
aa6076d
to
ec1211f
Compare
daviddavis
force-pushed
the
issue5559
branch
2 times, most recently
from
b4db1ab
to
56e3e95
Compare
daviddavis
changed the title
daviddavis
force-pushed
the
issue5559
branch
2 times, most recently
from
ca604a8
to
3b59bd4
Compare
gmbnomis
suggested changes
Nov 27, 2019
daviddavis
force-pushed
the
issue5559
branch
3 times, most recently
from
704edb4
to
63f1758
Compare
gmbnomis
reviewed
Nov 28, 2019
gmbnomis
reviewed
Nov 28, 2019
daviddavis
force-pushed
the
issue5559
branch
7 times, most recently
from
f4a89d7
to
2418088
Compare
daviddavis
force-pushed
the
issue5559
branch
3 times, most recently
from
eda7d3d
to
d2a9759
Compare
daviddavis
pushed a commit
to daviddavis/pulp_file
that referenced
this pull request
Dec 2, 2019
ref #5559 Required PR: pulp/pulpcore#420
daviddavis
pushed a commit
to daviddavis/pulp_file
that referenced
this pull request
Dec 2, 2019
ref #5559 Required PR: pulp/pulpcore#420
daviddavis
pushed a commit
to daviddavis/pulp_file
that referenced
this pull request
Dec 2, 2019
ref #5559 Required PR: pulp/pulpcore#420
bmbouter
approved these changes
Dec 3, 2019
gmbnomis
reviewed
Dec 3, 2019
pulpcore/app/files.py
Outdated
| prefix tree) to keep track of which paths we've already seen. | ||
|
|
||
| Args: | ||
| paths (list): A list of strings each representing a relative path |
There was a problem hiding this comment.
nitpick: this actually needs only an iterable of strings
daviddavis
force-pushed
the
issue5559
branch
2 times, most recently
from
b648bb2
to
e915cf7
Compare
gmbnomis
approved these changes
Dec 3, 2019
| with self.assertRaises(ValueError): | ||
| validate_file_paths(paths) | ||
|
|
||
| paths = ['a/b/c', 'a/b'] |
There was a problem hiding this comment.
Suggestion: Also include test cases in which an overlap adds multiple levels, e.g. ['a/b/c/d', 'a/b'] and ['a/b', 'a/b/c/d'].
I'm pretty certain that your implementation does not have problems with it, but it is a conceptually different case that needs to be caught.
|
I'm going to merge now to include in the next (and hopefully final) RC. @daviddavis are you able to send any final test changes in another PR? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #5559
https://pulp.plan.io/issues/5559
pulp_file PR: pulp/pulp_file#325