-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[WIP] remove tempname usage #9442
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
Changes from all commits
5cc1f5d
357d0ef
38703ef
0b8f130
c027102
4435efe
d385819
c5efd54
4db277d
9b0bcbd
aa16c9c
4719676
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -448,7 +448,7 @@ public function dumpFile($filename, $content, $mode = 0666) | |
throw new IOException(sprintf('Unable to write to the "%s" directory.', $dir), 0, null, $dir); | ||
} | ||
|
||
$tmpFile = tempnam($dir, basename($filename)); | ||
$tmpFile = self::tempnam2($dir, basename($filename)); | ||
|
||
if (false === @file_put_contents($tmpFile, $content)) { | ||
throw new IOException(sprintf('Failed to write file "%s".', $filename), 0, null, $filename); | ||
|
@@ -471,4 +471,37 @@ private function toIterator($files) | |
|
||
return $files; | ||
} | ||
|
||
/** adapted from: http://www.deltascripts.com/board/viewtopic.php?id=7843 */ | ||
public static function tempnam2($dir, $prefix, $postfix='') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @GromNaN I tried to look on alternatives http://www.php.net/manual/en/function.function-exists.php#108018 but the problem is GAE does not specify how to fetch functions that are permanently disabled, the logic also could become so complex that is going to get worst. Any further ideas? Worst case scenario we would have a specific class for GAE. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or we could rely on environment variable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that is a good idea! @fabpot ? |
||
{ | ||
if (!getenv('DISABLE_FUNCTIONS')) { | ||
return tempnam($dir, $prefix); | ||
} | ||
|
||
if ($dir[strlen($dir) - 1] == '/') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. realpath olnly works on filesystem. see https://bugs.php.net/bug.php?id=52769 |
||
$trailing_slash = ""; | ||
} else { | ||
$trailing_slash = "/"; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This ugly as hell. Why not just use |
||
|
||
/* The PHP function is_dir returns true on files that have no extension. | ||
The file type function will tell you correctly what the file is */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is wrong (at least in modern PHP versions). |
||
if (!is_dir(realpath($dir)) || filetype(realpath($dir)) != "dir") { | ||
// The specified dir is not actually a dir | ||
return false; | ||
} | ||
if (!is_writable($dir)) { | ||
// The directory will not let us create a file there | ||
return false; | ||
} | ||
|
||
do { | ||
$seed = substr(md5(microtime().posix_getpid()), 0, 8); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
$filename = $dir.$trailing_slash.$prefix.$seed.$postfix; | ||
} while (file_exists($filename)); | ||
touch($filename); | ||
|
||
return $filename; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this introduces a dependency to the Filesystem component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only feasible way, the other is to duplicate the function tempnam2 code into each component, did not think it was the DRY way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, it could be fine (let's wait to see what @fabpot thinks about it), but it needs to be reflected in the composer.json file