| Project: | ProjectPier |
| Version: | 0.8.0.3 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | phpfreak |
| Status: | closed - fixed |
The is_valid_url() function is poor, resulting in several hard-to-diagnose problems when certain perfectly valid URLs become involved (such as ProjectPier installations in a local network same-domain environment). The URL_FORMAT regex it uses is inadequate.
The following valid URL examples are currently considered "valid":
http://localhost/a/b/c
http://example.com/public/assets/javascript/widgets/UserBoxMenu/widget.css
The following (valid) URLs are currently considered "invalid" by the function:
http://192.168.4.2/projects/public/assets/javascript/widgets/UserBoxMenu/widget.css
http://projectserver/projects/public/assets/javascript/example.css
The following invalid URLs are also considered "valid" by the function!:
http://example.com:0/dummy%folder:/ / /haxx0r\your0box?%$%$""£$$("*£_%%%%?%?%?
Effects of this is_valid_url() code include bad stylesheet HTML generated on all pages when ProjectPier is running on certain networks:
In the apache error.log this appears as:
[Wed Jan 06 14:32:02 2010] [error] [client 192.168.1.2] File does not exist: /var/www/projects/public/assets/themes/azul/stylesheets/http:, referer: http://canyonero/projects/index.php?c=task&a=view_list&id=1&active_project=1
[Wed Jan 06 14:32:05 2010] [error] [client 192.168.1.2] File does not exist: /var/www/projects/public/assets/themes/azul/stylesheets/http:, referer: http://canyonero/projects/index.php?c=task&a=view_list&id=1&active_project=1
[Wed Jan 06 14:32:10 2010] [error] [client 192.168.1.2] File does not exist: /var/www/projects/public/assets/themes/azul/stylesheets/http:, referer: http://canyonero/projects/index.php?c=dashboard&a=index&active_project=1
[Wed Jan 06 14:32:16 2010] [error] [client 192.168.1.2] File does not exist: /var/www/projects/public/assets/themes/azul/stylesheets/http:, referer: http://canyonero/projects/index.php?c=task&a=view_list&id=2&active_project=3
[Wed Jan 06 14:32:20 2010] [error] [client 192.168.1.2] File does not exist: /var/www/projects/public/assets/themes/azul/stylesheets/http:, referer: http://canyonero/projects/index.php?c=task&a=view_list&id=2&active_project=3
[Wed Jan 06 14:32:21 2010] [error] [client 192.168.1.2] File does not exist: /var/www/projects/public/assets/themes/azul/stylesheets/http:, referer: http://canyonero/projects/index.php?c=project&a=index&active_project=3
[Wed Jan 06 14:32:22 2010] [error] [client 192.168.1.2] File does not exist: /var/www/projects/public/assets/themes/azul/stylesheets/http:, referer: http://canyonero/projects/index.php?c=dashboard&a=index&active_project=3
[Wed Jan 06 14:39:51 2010] [error] [client 192.168.1.2] File does not exist: /var/www/projects/public/assets/themes/azul/stylesheets/http:, referer: http://canyonero/projects/index.php?c=dashboard&a=index&active_project=3
[Wed Jan 06 14:44:06 2010] [error] [client 192.168.1.2] File does not exist: /var/www/projects/public/assets/themes/azul/stylesheets/http:, referer: http://canyonero/projects/index.php?c=dashboard&a=index&active_project=3
[Wed Jan 06 14:48:45 2010] [error] [client 192.168.1.2] File does not exist: /var/www/projects/public/assets/themes/azul/stylesheets/http:, referer: http://canyonero/projects/index.php?c=dashboard&a=my_projects&active_project=3
The issue described is also responsible for this bug:
http://www.projectpier.org/node/991
Patch to follow.
This single file patch against environment/constants.php (in version 0.8.0.3) improves URL validation, resolving the problems mentioned.
The change is to replace URL_FORMAT with:
define('URL_FORMAT',
'/^(https?):\/\/'. // protocol
'(([a-z0-9$_\.\+!\*\'\(\),;\?&=-]|%[0-9a-f]{2})+'. // username
'(:([a-z0-9$_\.\+!\*\'\(\),;\?&=-]|%[0-9a-f]{2})+)?'. // password
'@)?(?#'. // auth requires @
')((([a-z0-9][a-z0-9-]*[a-z0-9]\.)*'. // domain segments AND
'[a-z][a-z0-9-]*[a-z0-9]'. // top level domain OR
'|((\d|[1-9]\d|1\d{2}|2[0-4][0-9]|25[0-5])\.){3}'.
'(\d|[1-9]\d|1\d{2}|2[0-4][0-9]|25[0-5])'. // IP Address
')(:\d+)?'. // port
')(((\/+([a-z0-9$_\.\+!\*\'\(\),;:@&=-]|%[0-9a-f]{2})*)*'. // path
'(\?([a-z0-9$_\.\+!\*\'\(\),;:@&=-]|%[0-9a-f]{2})*)'. // query string
'?)?)?'. // path and query string optional
'(#([a-z0-9$_\.\+!\*\'\(\),;:@&=-]|%[0-9a-f]{2})*)?'. // fragment
'$/i');
The new regex is based on the URI specification. See: http://stackoverflow.com/questions/161738/what-is-the-best-regular-expression-to-check-if-...
Also attached are two standalone PHP validation tests (run from the php command line).
testold.php shows that only 2 out of 10 test URLs are validated correctly.
testnew.php shows that 10 out of 10 test URLs validate correctly with the new code.
I have verified that this patch fixes the "widget stylesheet href malformed in some environments" issue.
It is also now possible to remove the starts-with http://localhost hack in is_valid_url() as per the attached patch environment_functions_general_isvalidurl.patch.
I run my servers with just a host name or IP address so I did notice this error in the logs. I believe all of the issues are present in the 0.8.5 BETA 1 as well as svn177.
I have applied both of these patches to 0.8.5 BETA 1 and it does indeed resolve the "widget.css" error.
Great work. The URL format constant was also causing tests in the application to fail and then execute other code, revealing that certain code was missing :D. I was planning much time to fix this until I hit on this issue while going through the list of issues. Thanks a lot!!!
Accepted in 0.8.6