Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import uploader does not check Content-Disposition header #12455

Closed
EliasZ opened this issue Nov 27, 2017 · 5 comments
Closed

Import uploader does not check Content-Disposition header #12455

EliasZ opened this issue Nov 27, 2017 · 5 comments
Labels
Component: ImportExport Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@EliasZ
Copy link
Contributor

EliasZ commented Nov 27, 2017

Preconditions

Magento 2.2.1 (probably previous versions too, cannot imagine this functionality being removed on purpose)

Steps to reproduce

  1. Create a product import CSV with an image URL (which does not have a proper image extension) leading to an image being force downloaded by HTTP headers (for example: https://gist.github.com/brasofilo/2863355 (example gist))

  2. Import it

Expected result

  1. Magento properly checks the headers, downloads the file to the filename given in the headers and then imports it

Actual result

  1. Magento does not check the headers and downloads the file (for example http://example.com/downloadsomefile becomes something like /pub/media/import/httpexamplecomdownloadsomefile)
  2. The filename does not have a valid file extension and validation fails resulting in the file not being properly imported

Problem

Magento\CatalogImportExport\Model\Import\Uploader::move() sets $fileName to a stripped version of the URL. Here it should do a Magento\Framework\Filesystem\File\ReadInterface::stat() on the URL to check if the Content-Disposition header is set and a filename is provided.

@magento-engcom-team magento-engcom-team added Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Nov 27, 2017
@magento-engcom-team
Copy link
Contributor

@EliasZ, thank you for your report.
We've created internal ticket(s) MAGETWO-84645 to track progress on the issue.

@PieterCappelle
Copy link
Contributor

Should be fixed. If accepted I'll create backport to 2.2 & 2.1.

@magento-team
Copy link
Contributor

Hi @EliasZ. Thank you for your report.
The issue has been fixed in #12872 by @PieterCappelle in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming patch release.

@magento-team magento-team added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Jan 3, 2018
@EliasZ
Copy link
Contributor Author

EliasZ commented Jan 3, 2018

@PieterCappelle Did you verify this works with URLs sending a header detailing the filename? parse_url doesn't seem to check that.

@piotrekkaminski
Copy link
Contributor

This issue was moved to magento-engcom/import-export-improvements#78

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: ImportExport Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

7 participants