Skip to content
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

[false-positive] mkdir race conditions with literal and/or #23

Closed
marcelloh opened this issue Nov 2, 2016 · 5 comments
Closed

[false-positive] mkdir race conditions with literal and/or #23

marcelloh opened this issue Nov 2, 2016 · 5 comments

Comments

@marcelloh
Copy link

marcelloh commented Nov 2, 2016

if (!@mkdir($dir, 0777, true) and !is_dir($dir))
which is the same as
if (!@mkdir($dir, 0777, true) && !is_dir($dir))

but I use the more readable and.

But it blames me for the wrong reasons :-(

@kalessil kalessil changed the title False positive [false-positive] mkdir race conditions with literal and/or Nov 2, 2016
@kalessil
Copy link
Owner

kalessil commented Nov 2, 2016

Thanks for reporting, @marcelloh .

False-positive is confirmed, I will ping you when it fixed.

@dryabov
Copy link
Contributor

dryabov commented Nov 2, 2016

@kalessil I have false-positive for following code:

    public function makeDir($path, $chmod = 0777)
    {
        return is_dir($path) || @mkdir($path, $chmod, true) || is_dir($path);
    }

PS. And there is a conflict with "This condition is duplicated in other if/elseif branch" for similar code:

        if (!is_dir($destDir) && !@mkdir($destDir, 0755, true) && !is_dir($destDir)) {
            trigger_error("Cannot create directory $destDir");
        }

@marcelloh
Copy link
Author

marcelloh commented Nov 4, 2016

somehow when the order is the "adviced order"
if (!@mkdir($dir, 0777, true) and !is_dir($dir))
it still throws a warning or error or something: mkdir(): File exists

But when I change the order into:
if (!is_dir($dir) and !@mkdir($dir, 0777, true))
all is well, without any warning

@funivan
Copy link
Collaborator

funivan commented Nov 22, 2016

@dryabov Why you are using !is_dir before mkdir ?
mkdir is always executed because you are using && operator . So there is no point to check is_dir twice.

@kalessil kalessil added this to the 2.3.+ milestone Jul 6, 2017
@kalessil kalessil self-assigned this Sep 15, 2017
@kalessil kalessil added the fixed label Sep 15, 2017
@kalessil kalessil modified the milestones: 2.3.+, 2.3.10 Sep 15, 2017
@kalessil
Copy link
Owner

Fixed in 2.3.10, which I releasing today evening.

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

No branches or pull requests

4 participants