Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[DomCrawler] Keep temporary file copy filenames #6777

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

thewilkybarkid commented Jan 17, 2013

Q A
Bug fix? kind of
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

DomCrawler's FileFormField takes a copy of the file and gives it a new filename (on Windows something like upl8BF8.tmp). Using Goutte I've tried to upload an image which is rejected by the server (in this case Drupal) since it no longer has a .jpg filename. This PR changes the copy so that it goes into a unique path and keeps its filename. (It also adds tests to make sure the copy is actually there.)

@pborreli pborreli commented on the diff Jan 17, 2013

src/Symfony/Component/DomCrawler/Field/FileFormField.php
@@ -62,8 +62,12 @@ public function setValue($value)
$name = basename($value);
// copy to a tmp location
- $tmp = tempnam(sys_get_temp_dir(), 'upload');
- unlink($tmp);
+ $path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'symfony' . DIRECTORY_SEPARATOR . uniqid();
@pborreli

pborreli Jan 17, 2013

Contributor

you don't need to use DIRECTORY_SEPARATOR

@pborreli

pborreli Jan 17, 2013

Contributor

why are you creating 2 folders ?

@thewilkybarkid

thewilkybarkid Jan 17, 2013

Contributor

The former could be removed, it's superfluous.

@fabpot fabpot commented on the diff Mar 25, 2013

src/Symfony/Component/DomCrawler/Field/FileFormField.php
@@ -62,8 +62,12 @@ public function setValue($value)
$name = basename($value);
// copy to a tmp location
- $tmp = tempnam(sys_get_temp_dir(), 'upload');
- unlink($tmp);
+ $path = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'symfony' . DIRECTORY_SEPARATOR . uniqid();
+ mkdir($path, 0777, true);
+ $tmp = $path . DIRECTORY_SEPARATOR . $name;
@fabpot

fabpot Mar 25, 2013

Owner

If I understand correctly, you only need the extension, so I would just keep it and change the basename to a random one (just to avoid weird characters).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment