Files Action - Windows Paths Issue

[dartar]

(reported by QualousHere) On a Windows/IIS installation, including the {{files}} action causes errors similar to the following:

	Warning: mkdir(uploads\SandBox): Permission denied in D:\sitepath\wwwroot\wikka\actions\files.php on line 12
	Warning: opendir(uploads\SandBox): failed to open dir: Invalid argument in D:\sitepath\wwwroot\wikka\actions\files.php on line 156
	Warning: readdir(): supplied argument is not a valid Directory resource in D:\sitepath\wwwroot\wikka\actions\files.php on line 157
	Warning: closedir(): supplied argument is not a valid Directory resource in D:\sitepath\wwwroot\wikka\actions\files.php on line 200 

This is caused by the assumption of "/" as the directory seperator instead of Window's "\". Using the DIRECTORY_SEPARATOR variable corrects this issue. In actions/files.php change:

	$upload_path = $this->config['upload_path'].'/'.$this->GetPageTag();

into

	$upload_path = $this->config['upload_path'].DIRECTORY_SEPARATOR.$this->GetPageTag();

and also:

	$destfile = $upload_path.'/'.$strippedname;

into

	$destfile = $upload_path.DIRECTORY_SEPARATOR.$strippedname;

Warning: This has only been tested so far under Windows.

Related tickets

#72 #46

[BrianKoontz]

Tested under Unix/OS X, appears to work OK.

DIRECTORY_SEPARATOR is part of the Pear library. Is this a standard part of the PHP installation? Here's the functional expansion:

DIRECTORY_SEPARATOR = strtoupper(substr(PHP_OS,0,3)=='WIN')?'\\':'/'

[BrianKoontz]

Appears to have been fixed in [217].

[JavaWoman]

Working on #340 (Internationalization) and removing all markup from language strings, I came across error messages in the files action that still contained literal '/'. While that will not break anything, it may be confusing. Fixing those, too... done in [294].

Note: DIRECTORY_SEPARATOR is part of standard PHP - see:  http://php.net/manual/en/reserved.constants.php

[JavaWoman]

1.1.6.3: [402] applies [294]:[330] from trunk (with some conflicts resolution) for #312; also some initializations added (#38), slashes replaced by DIRECTORY_SEPARATOR (#89), and a lot of docblocks added or updated. (phew)

[JavaWoman]

also:
[394] applies DIRECTORY_SEPARATOR in diff.php
[396] applies DIRECTORY_SEPARATOR in files.xml.php

[JavaWoman]

(In [428]) implementing DIRECTORY_SEPARATOR - refs #89

[JavaWoman]

(In [429]) implementing DIRECTORY_SEPARATOR - refs #89

[JavaWoman]

(In [431]) Handles several problems related to IncludeBuffered(); there is no ticket for this, comments refer to [SEC] instead! Extra-long commit message to overcome lack of ticket info.

IncludeBuffered is used by Action(), Method() and Format(); the latter "calls" Action() (so IncludeBuffered() is "passed" twice when handling actions in the source). To make this complex more secure, all four methods need to be regarded (and including their interaction). Apart from comments, changes include:

1. Action()

• more direct parsing of the action code, combined with validation of syntactically-valid action name
• regard only parameters that actually have a name
• sanitize all parameters with htmlspecialchars_ent() (if an action really needs "raw" HTML input it's its own responsibility to unescape this)

2. Method()

• added a "syntax check" for method name, similar to that in Action(); the RE covers at least all current handlers
• implements DIRECTORY_SEPARATOR instead of '/' for constructing $methodLocation ( refs #89 )
• make Method name in URL case-sensitive by converting valid method name to lowercase (again, as with Action())

3. Format()

• added a "syntax check" for method name, similar to that in Action(); the RE covers at least all current formatters (and even those of GeSHi)
• make formatter name case-sensitive by converting valid formatter name to lowercase (again, as with Action())

4. IncludeBuffered()

• $path parameter (filled by the calling methods with a path specified in the configuration file) is no longer allowed to contain a list of paths, to enforce strict directory structure
• implements DIRECTORY_SEPARATOR instead of '/' for constructing path to file to be included ( refs #89 )
• added EXTR_SKIP parameter to extract() so (action) parameters can no longer override existing variables in the current scope
• if the path to the file to be included is not found, the provided error message (which may contain user-provided data such as a "hackish" path in a link) is sanitized before being returned for display

This is a "commented" version for extra clarification of what was observed, and then done; a cleanup version removing some of these will follow.

[DarTar]

Excellent job, thanks. I cannot test this right now, but I'll do later next week. Once this has been tested, we should port this code to trunk.

[DotMG]

(In [468]) refs #89. a little issue in the path (inopportune DIRECTORY_SEPARATOR)

[DarTar]

Mahefa, can you please give some explanations about the code changes committed in [468]? inopportune doesn't say much and I don't understand why this constant was removed, whereas it was introduced in [431].

Please every time you commit code, try to give detailed explanations about the reasons for a specific code change so that other developers can build on this change.

[DotMG]

Sorry! As one can see in the diff in [468], I removed leading DIRECTORY_SEPARATOR before $upload_path, ie replaced

'<tt>'.DIRECTORY_SEPARATOR.$upload_path.'</tt>'
with
'<tt>'.$upload_path.'</tt>'

The older version throws error message : Please make sure that the server has write access to a folder named /uploads; instead of Please make sure that the server has write access to a folder named uploads.

The $configupload_path?, as we defined it, is relative to wikka installation, not to root directory. The former version may mislead the user to create a folder named uploads at the root directory and give write access to it.

Hope that is clear enough, :)

[DarTar]

Shouldn't DIRECTORY_SEPARATOR be used anytime files are included via include/require ?

There's a require_once in Wakka.class.php (line 408) and two in wikka.php (line 88 & 130) that do not make use of DIRECTORY_SEPARATOR. I'm reopening this ticket accordingly.

[BrianKoontz]

(In [541]) Replaced more instances of Unix-style file separators with DIRECTORY_SEPARATOR define. Refs #89.

[DarTar]

Retargeting to 1.3, this ticket has already been closed in trunk, from which 1.3 will be branched. Consider backporting urgent issues to 1.2.X

[BrianKoontz]

Updated milestone to 1.3.1