Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rulesets cannot be loaded if the path contains urlencoded characters #1089

Merged
merged 1 commit into from
Jul 26, 2016
Merged

Rulesets cannot be loaded if the path contains urlencoded characters #1089

merged 1 commit into from
Jul 26, 2016

Conversation

hydrapolic
Copy link
Contributor

phpcs versions beyond 2.6.1 fail to process xml files in directories containing "%2F". This is because of this change:
322fa24

We discovered this because we build all our sources via Jenkins which transforms / into %2F so a directory looks like:
/home/jenkins/workspace/project/feature%2Fname

This is needed because in Linux0, / is a directory separator and cannot be used as part of the directory/file name.

phpcs will print an error:
Warning: simplexml_load_file(): I/O warning : failed to load external entity "/home/jenkins/workspace/project/feature%2Fname/phpcs.xml" in /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/CodeSniffer.php on line 715

Fatal error: Uncaught PHP_CodeSniffer_Exception: Ruleset /home/jenkins/workspace/project/feature%2Fname/phpcs.xml is not valid in /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/CodeSniffer.php:717
Stack trace:
#0 /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/CodeSniffer.php(562): PHP_CodeSniffer->processRuleset('/home/jenkins/w...')
#1 /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/CodeSniffer/CLI.php(907): PHP_CodeSniffer->initStandard(Array, Array, Array)
#2 /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/CodeSniffer/CLI.php(106): PHP_CodeSniffer_CLI->process()
#3 /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/scripts/phpcs(25): PHP_CodeSniffer_CLI->ru in /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/CodeSniffer.php on line 717

PHP Warning: simplexml_load_file(): I/O warning : failed to load external entity "/home/jenkins/workspace/project/feature%2Fname/phpcs.xml" in /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/CodeSniffer.php on line 715
PHP Fatal error: Uncaught PHP_CodeSniffer_Exception: Ruleset /home/jenkins/workspace/project/feature%2Fname/phpcs.xml is not valid in /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/CodeSniffer.php:717
Stack trace:
#0 /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/CodeSniffer.php(562): PHP_CodeSniffer->processRuleset('/home/jenkins/w...')
#1 /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/CodeSniffer/CLI.php(907): PHP_CodeSniffer->initStandard(Array, Array, Array)
#2 /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/CodeSniffer/CLI.php(106): PHP_CodeSniffer_CLI->process()
#3 /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/scripts/phpcs(25): PHP_CodeSniffer_CLI->ru in /home/jenkins/workspace/project/feature%2Fname/vendor/squizlabs/php_codesniffer/CodeSniffer.php on line 717

Reference:
doctrine/orm#5705

@aik099
Copy link
Contributor

aik099 commented Jul 26, 2016

We discovered this because we build all our sources via Jenkins which transforms / into %2F so a directory looks like:
/home/jenkins/workspace/project/feature%2Fname

Your fix seems to be tailored just to undo result of urlencoding done by you.

phpcs versions beyond 2.6.1 fail to process xml files in directories containing "%2F". This is because of this change: 322fa24

Are you certain, that file_get_contents function naturally supports paths with url-encoded entities, like %2F?

@aik099
Copy link
Contributor

aik099 commented Jul 26, 2016

Are you certain, that file_get_contents function naturally supports paths with url-encoded entities, like %2F?

This might be a side effect of file_get_contents function, because it also can be used to download files from a given url and urls can contain %2F. Not sure if file_get_contents function would even consider doing internal url decoding of given path if in php.ini the allow_url_fopen setting will be disabled for security reasons.

@hydrapolic
Copy link
Contributor Author

hydrapolic commented Jul 26, 2016

@aik099, this affects any project in a regular path containing %2F. For example, if you clone PHP_CodeSniffer into /tmp/PHP_CodeSniffer_%2F_new and run the tests:

$ pwd
/tmp/PHP_CodeSniffer_%2F_new

$ php scripts/phpcs . --standard=PHPCS -np
PHP Warning: simplexml_load_file(): I/O warning : failed to load external entity "/tmp/PHP_CodeSniffer_%2F_new/CodeSniffer/Standards/PHPCS/ruleset.xml" in /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer.php on line 714

Warning: simplexml_load_file(): I/O warning : failed to load external entity "/tmp/PHP_CodeSniffer_%2F_new/CodeSniffer/Standards/PHPCS/ruleset.xml" in /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer.php on line 714
PHP Fatal error: Uncaught PHP_CodeSniffer_Exception: Ruleset /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer/Standards/PHPCS/ruleset.xml is not valid in /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer.php:716
Stack trace:
#0 /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer.php(562): PHP_CodeSniffer->processRuleset('/tmp/PHP_CodeSn...')
#1 /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer/CLI.php(910): PHP_CodeSniffer->initStandard(Array, Array, Array)
#2 /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer/CLI.php(106): PHP_CodeSniffer_CLI->process()
#3 /tmp/PHP_CodeSniffer_%2F_new/scripts/phpcs(25): PHP_CodeSniffer_CLI->runphpcs()
#4 {main}
thrown in /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer.php on line 716

Fatal error: Uncaught PHP_CodeSniffer_Exception: Ruleset /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer/Standards/PHPCS/ruleset.xml is not valid in /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer.php:716
Stack trace:
#0 /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer.php(562): PHP_CodeSniffer->processRuleset('/tmp/PHP_CodeSn...')
#1 /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer/CLI.php(910): PHP_CodeSniffer->initStandard(Array, Array, Array)
#2 /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer/CLI.php(106): PHP_CodeSniffer_CLI->process()
#3 /tmp/PHP_CodeSniffer_%2F_new/scripts/phpcs(25): PHP_CodeSniffer_CLI->runphpcs()
#4 {main}
thrown in /tmp/PHP_CodeSniffer_%2F_new/CodeSniffer.php on line 716

@aik099
Copy link
Contributor

aik099 commented Jul 26, 2016

According to documentation page (see http://php.net/manual/en/function.simplexml-load-file.php) since PHP 5.1 the simplexml_load_file would internally do the urldecode on the path given in the filename. That explains why we're having this problem in the first place.

@hydrapolic
Copy link
Contributor Author

Do you suggest opening a bug on PHP itself?

@aik099
Copy link
Contributor

aik099 commented Jul 26, 2016

Nope. I've just found out documented behavior in PHP that explains why it happens. Opening bug in PHP won't do us any good, because it will be fixed only in PHP 7.0.0.

@hydrapolic
Copy link
Contributor Author

Can you please share your findings and maybe propose some other solution?

@aik099
Copy link
Contributor

aik099 commented Jul 26, 2016

Can you please share your findings and maybe propose some other solution?

Already did in #1089 (comment). I haven't copy-pasted relevant fragment from documentation, but just added a link to PHP website.

2016-07-26_1408

Your solution does exactly what's needed, even though code looks very unusual for anyone without knowledge of that problem.

@gsherwood
Copy link
Member

When I read those docs, it suggests that from PHP 5.1.0, PHP should be taking care of the encoding for you so you don't need to wrap every call to simplexml_load_file() with a rawurlencode(), but this clearly isn't happening. I've tested on my local system and get the same result.

@gsherwood
Copy link
Member

Note that this PR only changes one of the lines where this function is used, so I'll do it to the other as well.

@gsherwood gsherwood changed the title Encode path before calling simplexml_load_file() Rulesets cannot be loaded if the path contains urlencoded characters Jul 26, 2016
@gsherwood gsherwood merged commit c00d32c into squizlabs:master Jul 26, 2016
gsherwood added a commit that referenced this pull request Jul 26, 2016
@gsherwood
Copy link
Member

Thanks for finding and fixing this.

@gsherwood
Copy link
Member

I had to revert this change because HHVM completely failed, but worked on all standard PHP versions. I ended up putting back the old code that used simplexml_load_string() and file_get_contents() because it works everywhere.

Thanks for reporting the issue.

@hydrapolic hydrapolic deleted the load_special_dirs branch July 27, 2016 03:41
@hydrapolic
Copy link
Contributor Author

Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants