10000 [Filesystem] Fix mirroring a directory into itself or in his child with realpath checks by XuruDragon · Pull Request #30116 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Filesystem] Fix mirroring a directory into itself or in his child with realpath checks #30116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Mar 31, 2019
Merged

[Filesystem] Fix mirroring a directory into itself or in his child with realpath checks #30116

merged 2 commits into from
Mar 31, 2019

Conversation

XuruDragon
Copy link
@XuruDragon XuruDragon commented Feb 8, 2019 •
edited
Loading
Q A
Branch? master
Bug fix? yes
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets none / PR #29857
License MIT
Doc PR n/a

This this the continuity of #29857 by @Fleuv

Fix a bug while trying to mirror a directory into itself or in a child
Adding handle real path checks when mirroring.

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Feb 8, 2019
@XuruDragon XuruDragon force-pushed the fix-filesystem-mirroir branch 3 times, most recently from 12c07b8 to 55e8d21 Compare February 8, 2019 15:11
@nicolas-grekas nicolas-grekas mentioned this pull request Feb 20, 2019
Closed
@XuruDragon XuruDragon force-pushed the fix-filesystem-mirroir branch 3 times, most recently from fe32a0a to 785dc05 Compare February 22, 2019 10:33
@XuruDragon XuruDragon force-pushed the fix-filesystem-mirroir branch 6 times, most recently from f80d827 to a3efb95 Compare March 13, 2019 12:10
@XuruDragon
Copy link
Author
XuruDragon commented Mar 13, 2019 •
edited
Loading

Add handle for relative paths.

CI build fails not related

@XuruDragon XuruDragon force-pushed the fix-filesystem-mirroir branch 3 times, most recently from e174f37 to 5256eaa Compare March 14, 2019 09:22
@XuruDragon
Copy link
Author
XuruDragon commented Mar 14, 2019 •
edited
Loading

Added a bypass for symlink mirror tests on windows

CI build fails not related

@fabpot
Copy link
Member
fabpot commented Mar 24, 2019

I think this PR does not qualify for being a bug fix but rather as a new feature.

@XuruDragon
Copy link
Author
XuruDragon commented Mar 26, 2019 •
edited
Loading

Hi @fabpot , did you really think this PR should be qualify as a new feature ?

Since the behavior of mirror doesn't match what should be done at the origin (ie not to copy the target directory to the inside of itself).

Indeed, adding absolute path management to mirror might justify qualifying this PR as a new feature, but I think it was just a foregone departure.

@fabpot
Copy link
Member
fabpot commented Mar 27, 2019

I still think this is a new feature as the use case itself was not supported at all.

@XuruDragon
Copy link
Author

Ok So I'll rebase this branch on master and change de PR.

@XuruDragon XuruDragon force-pushed the fix-filesystem-mirroir branch from 5256eaa to dd2d409 Compare March 27, 2019 09:43
@XuruDragon XuruDragon changed the base branch from 3.4 to master March 27, 2019 09:43
@XuruDragon XuruDragon changed the title [Filesystem] Fix trying to mirror a directory into itself or in his child [Filesystem] Fix mirroring a directory into itself or in his child with realpath checks Mar 27, 2019
@Fleuv
Skipping iterations in the mirror() method where the iterated file's …
59437a4 
…path name is equal to the target path

Added a new test what is called testMirrorAvoidCopyingTargetDirectoryIfInSourceDirectory

Adjustments to comply with the Symfony Code Standards
@XuruDragon XuruDragon force-pushed the fix-filesystem-mirroir branch from dd2d409 to b312b7c Compare March 27, 2019 09:49
@XuruDragon XuruDragon force-pushed the fix-filesystem-mirroir branch from b312b7c to 8011f49 Compare March 27, 2019 09:50
@XuruDragon
Copy link
Author
XuruDragon commented Mar 27, 2019 •
edited
Loading

@fabpot as you asked for, I've rebase this PR on master and qualify it as a new feature.

@fabpot
Copy link
Member
fabpot commented Mar 31, 2019

Thank you @XuruDragon.

@fabpot fabpot merged commit 8011f49 into symfony:master Mar 31, 2019
fabpot added a commit that referenced this pull request Mar 31, 2019
@fabpot
feature #30116 [Filesystem] Fix mirroring a directory into itself or …
e92c120 
…in his child with realpath checks (Fleuv, XuruDragon)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[Filesystem] Fix mirroring a directory into itself or in his child with realpath checks

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | none / PR #29857
| License       | MIT
| Doc PR        | n/a

This this the continuity of #29857 by @Fleuv

Fix a bug while trying to mirror a directory into itself or in a child
Adding handle real path checks when mirroring.

Commits
-------

8011f49 Handling relative/absolute path
59437a4 Skipping iterations in the mirror() method where the iterated file's path name is equal to the target path
@fabpot fabpot mentioned this pull request May 9, 2019
Merged
@xabbuh xabbuh modified the milestones: 3.4, 4.3 Jun 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@dunglas dunglas Awaiting requested review from dunglas

@lyrixx lyrixx Awaiting requested review from lyrixx

@sroze sroze Awaiting requested review from sroze

@xabbuh xabbuh Awaiting requested review from xabbuh

Assignees
No one assigned
Labels
Bug Filesystem Status: Reviewed
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
6 participants
@XuruDragon @fabpot @nicolas-grekas @xabbuh @carsonbot @Fleuv
0