Skip to content

Conversation

@jrfnl
Copy link
Member

@jrfnl jrfnl commented Dec 7, 2020

Proposed Changes

While looking into the (non-)race condition described in #125, I noticed a potentially related issue.

As per the documentation of realpath():

realpath() returns FALSE on failure, e.g. if the file does not exist.

In the rare case that realpath() would return false within this plugin, false would subsequently be passed on to is_dir() and is_readable() which both expect a string path.

This tiny change makes the condition fail earlier by checking the return value of realpath().

While looking into the race condition described in 125, I noticed a potentially related issue.

As per the documentation of [realpath()](https://www.php.net/manual/en/function.realpath.php#refsect1-function.realpath-returnvalues):
> realpath() returns FALSE on failure, e.g. if the file does not exist.

In the rare case that `realpath()` _would_ return `false` within this plugin, `false` would subsequently be passed on to `is_dir()` and `is_readable()` which both expect a string path.

This tiny change make the condition fail earlier by checking the return value of `realpath()`.
@jrfnl
Copy link
Member Author

jrfnl commented Dec 8, 2020

@Potherca Just checking: where there any changed you wanted ? Or did you assign this to me to self-merge ?
(sorry, the commit process in this repo still confuses me - I'm used to never self-merge when collaborating, i.e. the (last) approver merges)

@Potherca Potherca merged commit c4a47f7 into master Dec 8, 2020
@Potherca Potherca deleted the feature/realpath-can-return-false branch December 8, 2020 15:46
@Potherca
Copy link
Member

Potherca commented Dec 8, 2020

No changes needed. I just didn't feel like waiting around for Travis to complete but also didn't want to use my admin right to merge again. I assigned you in case I forgot to come back to this 😂

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

Development

Successfully merging this pull request may close these issues.

3 participants