Problem/Motivation
In #1083982 it was agreed that drupal_realpath()
should generally not be used. A patch has been submitted to describe why it should not be used, and catch has suggested that we could go a step further here and mark drupal_realpath() deprecated maybe.
checkPath()
uses drupal_realpath()
where it should use realpath()
.
The code says:
$full_path = drupal_realpath(substr($this->chroot . $path, 0, strlen($full_jail)));
The prefix $this->chroot
is set as follows:
setChroot()
callsfindChroot()
findChroot()
starts with either__FILE__
or__DIR__
and trims leading parts, then appends a trailing'/'
character.
So the argument to drupal_realpath()
is guaranteed to be a local filesystem path, not a stream wrapper URI.
Removing the unnecessary use of a discouraged (and possibly deprecated) function helps set a good example for contrib module code.
Proposed resolution
The patch under review replaces drupal_realpath()
by realpath() and adds a justifying comment.
Remaining tasks
The patch in #1 needs to be reviewed, approved, and committed.
User interface changes
None.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#1 | FileTransfer_checkPath-remove-drupal_realpath-1283502-1.patch | 1.4 KB | pillarsdotnet |
Comments
Comment #1
pillarsdotnet CreditAttribution: pillarsdotnet commentedPatch.
Comment #1.0
pillarsdotnet CreditAttribution: pillarsdotnet commentedUpdated status.
Comment #1.1
pillarsdotnet CreditAttribution: pillarsdotnet commentedCorrected error in formatting.
Comment #2
kscheirer#1: FileTransfer_checkPath-remove-drupal_realpath-1283502-1.patch queued for re-testing.
Comment #3.0
(not verified) CreditAttribution: commentedMissing space.
Comment #10
kim.pepperClosing as outdated. Please re-open if this is still relevant.