HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Resolving directory traversal

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
directoryresolvingtraversal

Problem

I keep seeing /subsite/../global/css/file.css when I look at my website's source code, and today I decided that I want to get rid of the unnecessary traversal in the path itself, so it becomes /global/css/file.css. I made myself a function which I now use for all paths. Here is the base of my function that accomplishes this:

function djpth($pth){
    // Split path at slashes (always receives path with forward slash)
    $pth = explode('/',$pth);
    // Iterate through the new array
    foreach ($pth as $i => $part)
        // If we find a traversal mark
        if ($part == '..')
            // We get rid of the folder name before the mark and the mark itself
            array_splice($pth,$i-1,2);
    // We join the array back together
    $pth = implode('/',$pth);
    // Then we return the resolved path
    return $pth;
}


I want to know if there's a better way to go about this, or if this is the best I can get.

Solution

-
djpth('/parent/subsite/../../global/css/file.css')


returns

/parent/../file.css


I guess that is not what you want.

-
If the file exists I'd use the built-in realpath function. I guess it handles corner case like this and another ones better.

(See also: Effective Java, 2nd edition, Item 47: Know and use the libraries)

-
$pth is used for multiple purposes. It would be easier to read a $path and a $path_array.

-
Abbreviations are hard to read, like $pth. Longer names would make the code more readable since readers don't have to decode the abbreviations every time and when they write/maintain the code don't have to guess which abbreviation the author uses.

Furthermore, it's hard to pronounce (as well as djpth).


If you can’t pronounce it, you can’t discuss it without sounding like an idiot. “Well,
over here on the bee cee arr three cee enn tee we have a pee ess zee kyew int, see?” This
matters because programming is a social activity.

Source: Clean Code by Robert C. Martin, Use Pronounceable Names, p21

-
Comments like this are just noise:

// Iterate through the new array


It's obvious from the code itself. (Clean Code by Robert C. Martin: Chapter 4: Comments, Noise Comments)

-

$pth = implode('/',$pth);
// Then we return the resolved path
return $pth;


The comment above could be eliminated with a better variable name:

$resolved_path = implode('/', $path_array);
return $resolved_path;

Code Snippets

djpth('/parent/subsite/../../global/css/file.css')
/parent/../file.css
// Iterate through the new array
$pth = implode('/',$pth);
// Then we return the resolved path
return $pth;
$resolved_path = implode('/', $path_array);
return $resolved_path;

Context

StackExchange Code Review Q#43734, answer score: 4

Revisions (0)

No revisions yet.