Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Filesystem] Fix mirroring a directory with a relative path and a custom iterator#26771

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.

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

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfromfxbt:Fix-relative-path
Oct 10, 2018

Conversation

@fxbt
Copy link

@fxbtfxbt commentedApr 3, 2018
edited
Loading

QA
Branch?2.8 up to 4.1
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets
LicenseMIT
Doc PR

The Filesystem::mirror method with a Finder iterator doesn't work if the origin directory is relative.

This is a case where it won't work:

$dir = '/data/../data/';$finder = (new Finder())->in($dir)->getIterator();(new Filesystem())->mirror($dir, '/tmp', $finder);

The finder will return file objects like this :

SplFileInfo {    pathname: "/data/file.tmp"    ...}

But the following line will fail because because the$file->getPathname() and the$originDir are differents.

$target = $targetDir.substr($file->getPathname(), $originDirLen);// $file->getPathname() = '/data/file.tmp'// $originDirLen = strlen('/data/../data/') = 14// $target = '/tmp' instead of '/tpm/file.tpm'

In some case, it's even worse. If the filename length is bigger than the$originDirLen, the target file will be a file with a completely wrong name:

// $file->getPathname() = '/data/file123456789.tmp'// $originDirLen = strlen('/data/../data/') = 14// $target = '/tmp/56789.tmp' instead of '/tpm/file123456789.tmp'

I fixed this on my side by using the realpath function everytime i'm calling the mirror method, but i doubt this is the desired behavior.

@nicolas-grekasnicolas-grekas added this to the2.7 milestoneApr 3, 2018
@fxbtfxbtforce-pushed theFix-relative-path branch 5 times, most recently from5e288ac to26d858fCompareApril 9, 2018 07:20
@fxbtfxbt changed the title[Filesystem] Fix mirroring a directory with a relative path[Filesystem] Fix mirroring a directory with a relative path and a custom iteratorApr 9, 2018
@fxbtfxbtforce-pushed theFix-relative-path branch 2 times, most recently from5147080 to2799f3fCompareApril 18, 2018 08:48
@fxbtfxbtforce-pushed theFix-relative-path branch 2 times, most recently from835bdfc to748e3f7CompareApril 27, 2018 09:54
@fxbtfxbtforce-pushed theFix-relative-path branch 2 times, most recently from9f14b18 to855d866CompareMay 14, 2018 08:31
@fabpotfabpot modified the milestones:2.7,2.8May 28, 2018
@fxbtfxbtforce-pushed theFix-relative-path branch 2 times, most recently from03d4673 to023704eCompareMay 28, 2018 08:59
@fxbtfxbt changed the base branch from2.7 to2.8May 28, 2018 09:00
@fabpot
Copy link
Member

fabpot commentedJul 18, 2018
edited
Loading

We've removed usage orrealpath in the past for various reasons. I don't think reintroducing here is a good idea.

@fxbt
Copy link
Author

I wasn't aware of this, but it makes sense.

What do you think the correct approach is ? Throwing an exception to avoid mirroring a bunch of useless data ?

@fabpot
Copy link
Member

Throwing might be a good idea (callingrealpath in user's code is indeed a better "fix").

@fxbtfxbtforce-pushed theFix-relative-path branch 2 times, most recently from91bdfb7 toad7062cCompareJuly 19, 2018 09:41
@fxbt
Copy link
Author

An exception is now thrown and I tried to be explicit with the error message.

The fabbot failure is a false negative.

@nicolas-grekas
Copy link
Member

(could you please rebase on latest 2.8 and fix the CS issues fabbot is reporting?)

@fxbt
Copy link
Author

Done!

Copy link
Member

@fabpotfabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM, I'm going to merge it in master.

@fabpotfabpot changed the base branch from2.8 tomasterOctober 10, 2018 11:37
@fabpot
Copy link
Member

Thank you@fxbt.

@fabpotfabpot merged commit27b673c intosymfony:masterOct 10, 2018
fabpot added a commit that referenced this pull requestOct 10, 2018
… path and a custom iterator (fxbt)This PR was submitted for the 2.8 branch but it was squashed and merged into the 4.2-dev branch instead (closes#26771).Discussion----------[Filesystem] Fix mirroring a directory with a relative path and a custom iterator| Q             | A| ------------- | ---| Branch?       | 2.8 up to 4.1| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets || License       | MIT| Doc PR        |The Filesystem::mirror method with a Finder iterator doesn't work if the origin directory is relative.This is a case where it won't work:```$dir = '/data/../data/';$finder = (new Finder())->in($dir)->getIterator();(new Filesystem())->mirror($dir, '/tmp', $finder);```The finder will return file objects like this :```SplFileInfo {    pathname: "/data/file.tmp"    ...}```But the following line will fail because because the `$file->getPathname()` and the `$originDir` are differents.```$target = $targetDir.substr($file->getPathname(), $originDirLen);// $file->getPathname() = '/data/file.tmp'// $originDirLen = strlen('/data/../data/') = 14// $target = '/tmp' instead of '/tpm/file.tpm'```In some case, it's even worse. If the filename length is bigger than the `$originDirLen`, the target file will be a file with a completely wrong name:```// $file->getPathname() = '/data/file123456789.tmp'// $originDirLen = strlen('/data/../data/') = 14// $target = '/tmp/56789.tmp' instead of '/tpm/file123456789.tmp'```I fixed this on my side by using the realpath function everytime i'm calling the mirror method, but i doubt this is the desired behavior.Commits-------27b673c [Filesystem] Fix mirroring a directory with a relative path and a custom iterator
@fxbtfxbt deleted the Fix-relative-path branchOctober 16, 2018 14:12
This was referencedNov 3, 2018
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

2.8

Development

Successfully merging this pull request may close these issues.

4 participants

@fxbt@fabpot@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp