Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#34938 closed enhancement (fixed)

Allow filtering the temporary file name generated by wp_tempnam

Reported by: mihaimihai's profile mihaimihai Owned by: dd32's profile dd32
Milestone: 4.6 Priority: normal
Severity: normal Version: 4.4
Component: HTTP API Keywords:
Focuses: Cc:

Description

The wp_tempnam() function is located in wp-admin/includes/file.php

When you use download_url() to download a zip file from a url that has many GET parameters

e.g. http://example.com/?arg1=val1&arg2=val2&arg3=val3 ...... argn=valn,

wp_tmpnam() will generate a name for the temporary file from the list of args.

e.g. arg1val1arg2val2arg3val3......argn=valn.tmp

If this is long enough, on an Windows environment, it can weigh a lot to the limitation of maximum 256 chars for a path.

When copying the contents of the zip file to the wp-content/upgrade folder for instance if the path of the WordPress install + the folder name + any path inside the zip file is longer then 256 chars an error will occur. "Could not create directory"

Suggestion: allow the returned value of wp_tempnam() to be filtered. This will help with avoiding the long path issue with Windows

Attachments (1)

wp_tempnam-too-long-file-name.patch (793 bytes) - added by kollega007 9 years ago.

Download all attachments as: .zip

Change History (10)

#1 follow-up: @dd32
9 years ago

  • Milestone changed from Awaiting Review to Future Release

We won't be adding a filter here, but definitely should avoid long filenames where a real filename is not provided.

#2 in reply to: ↑ 1 @kollega007
9 years ago

Replying to dd32:

We won't be adding a filter here, but definitely should avoid long filenames where a real filename is not provided.

Hi Dion,

I'm an author on Envato marketplace and I have an issue related to this - Envato Toolkit plugin can't update our theme related to this :( ( Envato API url too long, so tmp file name is too long as well )

May I ask if this fix will be released in nearest versions? Is it included in a roadmap?

I've created a patch, if this helpful...

Please let me back, so I can understand what I able to say to my clients related to this issue.

Thanks,
Oleg

Last edited 9 years ago by kollega007 (previous) (diff)

#3 follow-up: @dd32
9 years ago

  • Milestone changed from Future Release to 4.6
  • Owner set to dd32
  • Status changed from new to accepted

I'll look into this this cycle.

@kollega007 The earliest it'll get fixed is in the 4.6 release, so later this year. You could work around it by serving shorter update URLs.

This ticket was mentioned in Slack in #core by rmccue. View the logs.


9 years ago

#5 in reply to: ↑ 3 @kollega007
9 years ago

Replying to dd32:

I'll look into this this cycle.

@kollega007 The earliest it'll get fixed is in the 4.6 release, so later this year. You could work around it by serving shorter update URLs.

I see, thanks.

Related to usage of shorter URLs - sorry, but I'm not a developer for Envato's API. This is a bug of WP function, so I suppose, it should be fixed on this side.

Thanks that let me back. Have a nice day!

#6 @DrewAPicture
9 years ago

@dd32 Sounds we're against adding the filter, so how would you proceed with a fix for this?

#7 follow-up: @dd32
9 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 37598:

Updates: Only use the filename component of URLs to form part of the temporary filename.
Previously we were passing the entire URL to wp_tempnam() (incorrectly) which caused the query string to be used as part of the temporary filename.
We now only use the file component of a url such as https://example.com/filename.zip?arg1=1&arg2=2....&arg100=100 to prevent a long filename.

Fixes #34938

#8 in reply to: ↑ 7 ; follow-up: @kollega007
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to dd32:

But what if we have http://example.com/arg1/1/arg2/2/.../arg100/100/filename.zip ?

In 37598:

Updates: Only use the filename component of URLs to form part of the temporary filename.
Previously we were passing the entire URL to wp_tempnam() (incorrectly) which caused the query string to be used as part of the temporary filename.
We now only use the file component of a url such as https://example.com/filename.zip?arg1=1&arg2=2....&arg100=100 to prevent a long filename.

Fixes #34938

#9 in reply to: ↑ 8 @dd32
9 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to kollega007:

Replying to dd32:

But what if we have http://example.com/arg1/1/arg2/2/.../arg100/100/filename.zip ?

basename() returns the final component of the path.

  • filename.zip in the case of http://example.com/arg1/1/arg2/2/.../arg100/100/filename.zip
  • filename.zip in the case of http://example.com/filename.zip?arg=1
  • directory in the case of http://example.com/directory/?arg1=1&filename=blah.zip

The last case is fine, as it's just a hint for the temporary file, having a generic string there isn't a massive issue.

Note: See TracTickets for help on using tickets.