Cleanup FileSystemLoader (Followup to #775) #785
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a quick followup to #775.
The call to
file_exists()
is redundant and no longer required, asrealpath()
will already return false if the path does not exist.Additionally, the call to
ltrim()
is no longer needed asrealpath()
will resolve the path properly, even if there are two slashes due to the root path and relative path concatenation.Lastly, the
ltrim()
call may have also previously been a security consideration that would disallow thedata_root
being set to an empty string and absolute paths being passed as the relative path tofind()
. This is mitigated by the new check in the constructor that ensuresdata_root
is not empty. A test for this edge-case is included.(If someone really wants to push the boundaries of acceptable security, they can still set
data_root
to/
and effectively pass absolute paths to thefind()
method, though this is a horrible idea... :-) )