magento-engcom / import-export-improvements

Open Software License 3.0
31 stars 29 forks source link

Tablerate->getCsvFile() fails with non-default PHP upload_tmp_dir #84

Closed piotrekkaminski closed 5 years ago

piotrekkaminski commented 6 years ago

From @zts on June 26, 2017 15:53

Preconditions

  1. PHP upload_tmp_dir is set to a directory outside /tmp/.
  2. Magento CE 2.1.x, code still in latest version here: https://github.com/magento/magento2/blob/develop/app/code/Magento/OfflineShipping/Model/ResourceModel/Carrier/Tablerate.php#L315

Steps to reproduce

  1. Make a new directory for uploads, eg mkdir -p /var/lib/php/uploads && chmod 1777 /var/lib/php/uploads
  2. Set PHP upload_tmp_dir to this directory. eg, add upload_tmp_dir = /var/lib/php/uploads to /etc/php.ini
  3. Login to Magento Admin
  4. Go to Stores > Configuration > Sales > Shipping Methods
  5. Select Table Rates, and enable
  6. Click "Import"
  7. Choose a file, click save.

Expected result

  1. Table rates are updated

Actual result

  1. Receive an error, eg The file “/tmp/var/lib/php/uploads/phphEpJe5” doesn’t exist

Cause

This behaviour occurs because getCsvFile opens the /tmp directory, then looks for the uploaded file relative to that directory. This only works when upload_tmp_directory is set to /tmp, or a subdirectory of /tmp.

Copied from original issue: magento/magento2#10058

piotrekkaminski commented 6 years ago

From @vityakopin on July 4, 2017 12:29

Internal ticket MAGETWO-70451 was created.

piotrekkaminski commented 6 years ago

From @magento-engcom-team on October 9, 2017 11:30

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

dmanners commented 6 years ago

This has been fixed already in 2.3 by @RomaKis do we want to consider it for a backport? @PieterCappelle @piotrekkaminski

PieterCappelle commented 6 years ago

Yes, backport is needed.

dmanners commented 6 years ago

@RomaKis would you like to work on this backport? The issue is assigned to you but I think that was because the original issue was assigned to you. If not I can un-assign this and someone from the project can pick this up.

RomaKis commented 6 years ago

@dmanners, pls un-assign me.

adam-paterson commented 6 years ago

@dmanners I can look into this one, I need to understand the process and effort required for back-porting for M2 though. Could you ping me on Slack when you have a minute to discuss. ❤️

dmanners commented 5 years ago

All covered here now. Will close this off.