5

The Problem

I'm having an issue with the PHP function is_file().

Some preliminaries: I'm developing on Ubuntu 12.04, 32-bit, using PHP 5.5.10 and Apache 2.4.9.

I'm currently rewriting some working code to convert it to a library in Laravel (completed with a Facade and a ServiceProvider). I'm doing this mainly to clean up some code I wrote when I was young and foolish (about 6 months ago) and to implement unit testing. The library I'm writing provides methods for taking a contract (of which there are two distinct types, with more to come) and finding the path to a PDF document (the scanned paper contract). My methods for finding the path work fine and the tests are all passing.

In my old code, I used to do this:

/**
 * Get a scanned contract and return it to the client
 *
 * @param string $type
 * The contract type. Must be either static::CONTRACT1 or static::CONTRACT2.
 * 
 * @param string $contract_id
 * The contract ID
 *
 * @return Response
 */
public static function get($type, $contract_id)
{
    // get the file name
    //
    $results = static::getFileName($type, $contract_id);

    // did we find a file? if not, throw a ScannedContractNotFoundException
    //
    if(!$results)
        throw new \MyApplication\Exceptions\ScannedContractNotFoundException("No file found for $type contract $contract_id");

    // get the path and file name
    //
    $path = $results['path'];
    $name = $results['name'];

    // get the full path
    //
    $file = $path.$name;

    // get the file size
    //
    $contents = file_get_contents($file);
    $fsize = strlen($contents);

    // push the file to the client
    //
    header("Content-type: application/pdf");
    header("Content-Disposition: inline; filename=\"".$name."\"");
    header("Content-length: $fsize");
    header("Cache-control: private");

    echo $contents;

    exit;
}

And it worked just fine.

Now I'm trying to rewrite it to get rid of the echo and move the code that actually does the work of sending the file to a controller. That code will look like this:

$x = \MyApplication\Models\Contract1::find($id);

$file = \ScannedContracts::getFileName($x);

$path = $file["path"].$file["name"];

return \Response::download($path, $file["name"]);

However, this code is throwing a FileNotFoundException. The code where the exception is being thrown looks like this:

public function __construct($path, $checkPath = true)
{
    if ($checkPath && !is_file($path)) {
        throw new FileNotFoundException($path);
    }

...

Clearly the problem is with the if statement, and in particular the call to is_file().

I have written a little script to test this with a path which is known to be good and is_file() returns false.

When I copy a file to my "public" folder, it works fine.

In the documentation for the is_file() function there is a comment stating that the permissions for the parent folder must be +x. I've examined the permissions, and the folder is world-executable, as is the parent, and the grand-parent, and the great-grand-parent, etc.

There are two possible confounding factors: first, the files I'm working with are located on a CIFS/Samba share. I should mention that the paths in question are absolute paths to the mounted share.

The closest issue to this I've found on SO is PHP is_file returns false (incorrectly) for Windows share on Ubuntu, but that doesn't have a resolution. I've also searched for PHP bug reports, but there's nothing.

Second, some of the paths contain spaces. I've tried escaping them every way I can think of, but it doesn't help.

In the absence of a solution, I'll have to do it the old fashioned way, but I really wanted to do it using the functions Laravel provides.

Questions

  1. Do I need to escape spaces in a path passed to is_file()?

  2. Does anyone know of a fix or a workaround that doesn't a) require changing code in a 3rd party library, or b) require wholesale changes to permissions or other configuration on the CIFS/Samba server?

Thanks in advance!

Community
  • 1
  • 1
Kryten
  • 15,230
  • 6
  • 45
  • 68
  • 1. -> yes - you cannot use spaces, replace spaces with `%20` -> other than that it should work as expected -> the only thing is if a file is anything other than a 'Regular file' it won't work - but I cannot see you trying to access 'Special Files' in this way anyway :-P – GrahamTheDev Apr 11 '14 at 18:19
  • Did you try in a cli to do `php -r 'var_dump(is_file("/path/to/your/win/share"));'` ? And by the way, Does apache has read access on this folder ? no open_base_dir restriction ? no safe_mode ? – naab Apr 11 '14 at 18:20
  • @naab it's a web application and, in answer to all your other questions about environment, I _am_ able to download the exact same file from the share using the old code. I did test `is_file("/path/to/my/win/share/file.pdf")` and it returned false. – Kryten Apr 11 '14 at 18:23
  • @GrahamRitchie "yes - you cannot use spaces" - `php -r 'var_dump(is_file("/tmp/toto a/a"));'` prints : bool(true) (and the file exists) – naab Apr 11 '14 at 18:25
  • @GrahamRitchie I tried replacing spaces in the path with both `%20` and `%020` - neither worked. – Kryten Apr 11 '14 at 18:25
  • @Kryten seems to be a bug in PHP : https://bugs.php.net/bug.php?id=62199 and https://bugs.php.net/bug.php?id=49620 . I wouldn't be surprised if `is_file()` is buggy too. – naab Apr 11 '14 at 18:31
  • @naab yeah, I saw those bugs too, but wasn't sure if they were relevant. Reading through some of the links, I see someone else commented that "`is_readable()` just wraps the access system call to determine the file permission, so it´s very likely not a php issue" (from http://stackoverflow.com/questions/10818770/php-is-readable-fails-on-readable-samba-directory). This looks like it goes much deeper than just PHP :-P Thanks for all your help, @naab & @GrahamRitche – Kryten Apr 11 '14 at 19:17

1 Answers1

1

I think you need to sanitize filenames when upload to directory

function sanitize_file_name( $str ) {
    return preg_replace("/[^a-z0-9\.]/", "", strtolower($str));
}
Asil Balaban
  • 96
  • 1
  • 10