8000 [Translation] CsvFileLoader accept URL by stealth35 · Pull Request #2327 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Translation] CsvFileLoader accept URL #2327

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
Closed
wants to merge 1 commit into from
Closed

[Translation] CsvFileLoader accept URL #2327

wants to merge 1 commit into from

Conversation

stealth35
Copy link
Contributor

Bug fix: yes
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -

@fabpot
Copy link
Member
fabpot commented Oct 5, 2011

I think we need to forbid non local stream.

@stealth35
Copy link
Contributor Author

@fabpot why ?

so like :

if (!stream_is_local($resource)) {
    throw new \InvalidArgumentException(sprintf('No local file "%s".', $resource));
}

@fabpot
Copy link
Member
fabpot commented Oct 5, 2011

Because this is a security issue and I don't see any valid use case where you would have to support external URLs.

@stealth35
Copy link
Contributor Author

@fabpot ok, so should we had the code in my previous comment in each Loader ?

@stealth35 stealth35 closed this Oct 5, 2011
@fabpot
Copy link
Member
fabpot commented Oct 5, 2011

yes, it probably makes sense.

fabpot added a commit that referenced this pull request Oct 8, 2011
@fabpot
merged branch stealth35/fix_distant_trans (PR #2346)
f75f466 
Commits
-------

ae0685a [Translation] Loader should only load local files

Discussion
----------

[Translation] Security : Loader should only load local files

Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
See: #2327
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stealth35 @fabpot
0