#55109 closed defect (bug) (fixed)
Plugins no longer download to tmp folder
Reported by: | antonynz | Owned by: | Clorith |
---|---|---|---|
Milestone: | 5.9.1 | Priority: | normal |
Severity: | normal | Version: | 5.9 |
Component: | Filesystem API | Keywords: | has-patch needs-testing fixed-major |
Focuses: | Cc: |
Description
5.9 introduces a content-disposition check in the download_URL function to update the filename externally.
However when this header is set, the temporary path is removed from the $tmpfname variable which is added earlier, i.e:
$tmpfname = wp_tempnam( $url_filename );
is overridden with:
$tmpfname = $tmpfname_disposition;
This results in plugins that are downloaded from the WordPress plugin repository (as they include the content-disposition header) being downloaded to the folder the user is in eg, wp-admin if updating via the plugins page or the home directory if updated via a cron auto-update job for example. The overriding of the variable also skips the writable directory checks within wp_tempnam.
I've read over the comments for the addition of this code, but it's not clear what the use case is or if it was intended to apply to plugins?
[https://github.com/WordPress/WordPress/commit/7a0a07d69156aba316d566f37dcdd498f59d138d
]
Attachments (1)
Change History (12)
#1
@
3 years ago
- Component changed from General to Filesystem API
- Milestone changed from Awaiting Review to 5.9.1
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
3 years ago
#4
@
3 years ago
- Keywords has-patch needs-testing added
Hi thanks for the report, the issue is due to php rename fonction, see https://www.php.net/manual/fr/function.rename.php#97347.
I have submitted a patch to ensure that the rename occurs to the original file dir.
#5
@
3 years ago
This seems like the simplest solution, but pinging @johnjamesjacoby since he spearheaded the original implementation, in case there's a consideration that was taken there that might be overlooked here.
#6
follow-up:
↓ 8
@
3 years ago
- Resolution set to fixed
- Status changed from accepted to closed
In 52734:
#7
@
3 years ago
- Keywords fixed-major added
- Resolution fixed deleted
- Status changed from closed to reopened
Re-opening for 5.9.1 backports
(note on comment:5, I walked through all the changes of the original ticket, and it seems the missing tmp path was an oversight as the initial tickets would add the path via helper functions)
#8
in reply to:
↑ 6
@
3 years ago
Replying to Clorith:
In 52734:
Hello @Clorith thanks for the commit. I notice that in [52734] you add some unit test, that's good, just one thing, in the test function docblock you reference the trac ticket like that:
* @ticket #55109
see: https://prnt.sc/26xt3yp;
I think it should be:
* @ticket 55109
the "#" should not be there I think. as per doc https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#annotations https://prnt.sc/26xtihw.
Let me know your thought. thanks.
Hi there, welcome to WordPress Trac! Thanks for the report.
Introduced in [51939] / #38231. Moving to the 5.9.1 milestone for visibility.