Skip to content

Commit 2dcb076

Browse files
minor symfony#60808 [HttpFoundation] Handle error when expected directory is a file (afiestas)
This PR was merged into the 7.4 branch. Discussion ---------- [HttpFoundation] Handle error when expected directory is a file | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | kinda | New feature? | no | Deprecations? | no | License | MIT [HttpFoundation] In File::getTargetFile the directory is recursively created if it does not exists. If directory creation fails a FileException is thrown. This commit improves error clarity by adding a new FileException for the relatively common case where the path exists but instead of a directory it is a file. Since errors from `@mkdir` are being suppressed we don't really know why the operation has failed. There are many ways of doing this depending on the warranties that Symfony gives on retro-compatibility for the thrown exceptions. I Haven't been able to find documentation about that so I've decided to open a PR with the easiest to read change to get the conversation started. Depending on your preferences we could: - Manage the mkdir errors similar to how is done in [Filesystem::box](https://github.com/symfony/filesystem/blob/7.3/Filesystem.php#L753). Perhaps add that extra information after the current message like "Unable to create the %s directory (error goes here). - Same thing as the PR but as part of the if/elseif block. - Move the is_file check within the error handling code path to prevent another I/O operation (so we do not affect the happy path) - Others? Also since I haven't been able to find the compatibility guidelines I'm doing this to the 7.4 branch. Commits ------- b1a9d58 [HttpFoundation] Handle error when directory is a file
2 parents e088d76 + b1a9d58 commit 2dcb076

File tree

1 file changed

+4
-3
lines changed
  • src/Symfony/Component/HttpFoundation/File

1 file changed

+4
-3
lines changed

src/Symfony/Component/HttpFoundation/File/File.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,11 @@ public function getContent(): string
114114

115115
protected function getTargetFile(string $directory, ?string $name = null): self
116116
{
117-
if (!is_dir($directory)) {
118-
if (false === @mkdir($directory, 0o777, true) && !is_dir($directory)) {
119-
throw new FileException(\sprintf('Unable to create the "%s" directory.', $directory));
117+
if (!is_dir($directory) && !@mkdir($directory, 0o777, true) && !is_dir($directory)) {
118+
if (is_file($directory)) {
119+
throw new FileException(\sprintf('Unable to create the "%s" directory: a similarly-named file exists.', $directory));
120120
}
121+
throw new FileException(\sprintf('Unable to create the "%s" directory.', $directory));
121122
} elseif (!is_writable($directory)) {
122123
throw new FileException(\sprintf('Unable to write in the "%s" directory.', $directory));
123124
}

0 commit comments

Comments
 (0)