UriRewriter is confused by many "../" traversals

GoogleCodeExporter commented 9 years ago

Minify version: 2.1.2
PHP version: PHP 5.2.4-2ubuntu5.5

What steps will reproduce the problem?
1. include a css rule with a relative path more going up more several
directories, such as: 
"/static/js/yui/container/assets/skins/sam/../../../../assets/skins/sam/sprite.p
ng"

2. Minify the css and inspect the output that has the rewritten uri, and
you get: "/static/js/yui/container/assets/assets/skins/sam/sprite.png"

Expected output: /static/js/yui/assets/skins/sam/sprite.png

Actual output: /static/js/yui/container/assets/assets/skins/sam/sprite.png

Did any unit tests FAIL?

I don't see any related tests failing, but here is the full output of
running the suite:

PASS: Minify : 304 response (1 of 1 tests run so far have passed)
PASS: Minify : encoding, cache, and minifier classes aren't loaded for 304s
(2 of 2 tests run so far have passed)
PASS: Minify : JS and Expires (3 of 3 tests run so far have passed)
PASS: Minify : Issue 73 (4 of 4 tests run so far have passed)
PASS: Minify : Issue 89 : bubbleCssImports (5 of 5 tests run so far have
passed)
PASS: Minify : Issue 89 : detect invalid imports (6 of 6 tests run so far
have passed)
PASS: Minify : Issue 89 : don't warn about valid imports (7 of 7 tests run
so far have passed)
PASS: Minify : CSS and Etag/Last-Modified (8 of 8 tests run so far have passed)
PASS: Minify_Build : single file path (9 of 9 tests run so far have passed)
PASS: Minify_Build : multiple file paths (10 of 10 tests run so far have
passed)
PASS: Minify_Build : file path and a Minify_Source (11 of 11 tests run so
far have passed)
PASS: Minify_Build : uri() with no querystring (12 of 12 tests run so far
have passed)
PASS: Minify_Build : uri() with existing querystring (13 of 13 tests run so
far have passed)
!FAIL: Minify_Cache_File : store (1 of 14 tests run so far have failed)
PASS: Minify_Cache_File : getSize (14 of 15 tests run so far have passed)
!FAIL: Minify_Cache_File : isValid (2 of 16 tests run so far have failed)
!FAIL: Minify_Cache_File : display (3 of 17 tests run so far have failed)
!FAIL: Minify_Cache_File : fetch (4 of 18 tests run so far have failed)
!FAIL: Minify_Cache_File : store w/ lock (5 of 19 tests run so far have failed)
PASS: Minify_Cache_File : getSize (15 of 20 tests run so far have passed)
!FAIL: Minify_Cache_File : isValid (6 of 21 tests run so far have failed)
!FAIL: Minify_Cache_File : display w/ lock (7 of 22 tests run so far have
failed)
!FAIL: Minify_Cache_File : fetch w/ lock (8 of 23 tests run so far have failed)
PASS: Minify_CSS : styles (16 of 24 tests run so far have passed)
PASS: Minify_CSS : vladmirated (17 of 25 tests run so far have passed)
PASS: Minify_CSS : paths_rewrite (18 of 26 tests run so far have passed)
PASS: Minify_CSS : selectors (19 of 27 tests run so far have passed)
PASS: Minify_CSS : paths_prepend (20 of 28 tests run so far have passed)
PASS: Minify_CSS : issue62 (21 of 29 tests run so far have passed)
PASS: Minify_CSS : hacks (22 of 30 tests run so far have passed)
PASS: Minify_CSS : unusual_strings (23 of 31 tests run so far have passed)
PASS: Minify_CSS : comments (24 of 32 tests run so far have passed)
PASS: Minify_CSS : subsilver (25 of 33 tests run so far have passed)
PASS: Minify_CSS_UriRewriter (26 of 34 tests run so far have passed)
PASS: Minify_CommentPreserver (27 of 35 tests run so far have passed)
PASS: Minify_CommentPreserver (28 of 36 tests run so far have passed)
PASS: Minify_CommentPreserver (29 of 37 tests run so far have passed)
PASS: Minify_CommentPreserver (30 of 38 tests run so far have passed)
PASS: Minify_HTML (31 of 39 tests run so far have passed)
PASS: Minify_HTML (32 of 40 tests run so far have passed)
PASS: ImportProcessor (33 of 41 tests run so far have passed)
PASS: ImportProcessor : included right files in right order (34 of 42 tests
run so far have passed)
PASS: Minify_Javascript (35 of 43 tests run so far have passed)
PASS: Minify_Javascript : Quotes in RegExp literals (Issue 74) (36 of 44
tests run so far have passed)
PASS: Minify_Lines (37 of 45 tests run so far have passed)
PASS: HTTP_Encoder : recognize "x-gzip" as gzip (38 of 46 tests run so far
have passed)
PASS: HTTP_Encoder : gzip w/ non-zero q (39 of 47 tests run so far have passed)
PASS: HTTP_Encoder : gzip w/ zero q (40 of 48 tests run so far have passed)
PASS: HTTP_Encoder : IE6 w/o "enhanced security" (41 of 49 tests run so far
have passed)
PASS: HTTP_Encoder : IE6 w/ "enhanced security" (42 of 50 tests run so far
have passed)
PASS: HTTP_Encoder : IE5.5 (43 of 51 tests run so far have passed)
PASS: HTTP_Encoder : Opera identifying as IE6 (44 of 52 tests run so far
have passed)
PASS: HTTP_Encoder : IE6 w/ "enhanced security" (45 of 53 tests run so far
have passed)
PASS: HTTP_Encoder : deflate : uncompress possible (46 of 54 tests run so
far have passed)
PASS: HTTP_Encoder : deflate : compressed to 30.14% of original (47 of 55
tests run so far have passed)
PASS: HTTP_Encoder : gzip : uncompress possible (48 of 56 tests run so far
have passed)
PASS: HTTP_Encoder : gzip : compressed to 30.16% of original (49 of 57
tests run so far have passed)
PASS: HTTP_Encoder : compress : uncompress possible (50 of 58 tests run so
far have passed)
PASS: HTTP_Encoder : compress : compressed to 30.20% of original (51 of 59
tests run so far have passed)
PASS: HTTP_ConditionalGet : client has valid If-Modified-Since (52 of 60
tests run so far have passed)
PASS: HTTP_ConditionalGet : client has valid If-Modified-Since with
trailing semicolon (53 of 61 tests run so far have passed)
PASS: HTTP_ConditionalGet : client has valid ETag (54 of 62 tests run so
far have passed)
PASS: HTTP_ConditionalGet : no conditional get (55 of 63 tests run so far
have passed)
PASS: HTTP_ConditionalGet : client has invalid ETag (56 of 64 tests run so
far have passed)
PASS: HTTP_ConditionalGet : client has invalid If-Modified-Since (57 of 65
tests run so far have passed)
PASS: environment : DOCUMENT_ROOT should not end in trailing slash (58 of
66 tests run so far have passed)
PASS: environment : DOCUMENT_ROOT should pass realpath() (59 of 67 tests
run so far have passed)
PASS: environment : DOCUMENT_ROOT should contain this test file (60 of 68
tests run so far have passed)
!FAIL: environment : PHP/server does not auto-HTTP-encode content (9 of 69
tests run so far have failed)

Please provide any additional information below.

I traced it to the preg_replace() call on line 179 of UriRewritter.  It
doesn't seem to handle the relative pathing when its as deep as the example
given.

Original issue reported on code.google.com by bmhar...@gmail.com on 18 Mar 2009 at 2:25

GoogleCodeExporter commented 9 years ago

Thanks. I'm refactoring this class right now so I'll look into that.

Original comment by mrclay....@gmail.com on 18 Mar 2009 at 2:44

Changed title: UriRewriter is confused by many "../" traversals
Changed state: Accepted
Added labels: Priority-High
Removed labels: Priority-Medium

GoogleCodeExporter commented 9 years ago

What are your docRoot and symlink settings?

Original comment by mrclay....@gmail.com on 18 Mar 2009 at 4:39

GoogleCodeExporter commented 9 years ago

Please update your files that were changed in R309, enable debug mode in 
config.php, 
and add &debug=1 to your minify CSS URI. This will prepend the output with 
debugging 
info for the URI rewriting process.

The new rewriter also removes trailing slashes from realpath()s, so it's 
possible 
the update could solve some issues.

Original comment by mrclay....@gmail.com on 19 Mar 2009 at 4:44

Changed state: NeedInfo

GoogleCodeExporter commented 9 years ago

I'm having the same problem also (with latest checkout from Trunk).
As I only use UriRewriter class and not the whole minify suite, how do I enable 
debug
mode so I can provide additional info?

Original comment by laya...@yahoo.com on 4 May 2009 at 8:37

GoogleCodeExporter commented 9 years ago

@laya... : If you're just using that class, just echo Minify_CSS_UriRewriter::
$debugText after calling rewrite() or rewriteRelative().

Original comment by mrclay....@gmail.com on 4 May 2009 at 8:59

GoogleCodeExporter commented 9 years ago

Thanks for the quick reply ;)
With the help of $debugText, I have fixed most of my problems (with one problem
remaining). Previously I was passing $path including the css file name, now I'm
passing just the path [with dirname($path)].

Info:
Trying to use Minify_CSS_UriRewriter::rewrite with YUI.
Doc Root: c:\projects\abc\web
Yui located in: c:\projects\abc\web\yui

CSS I was testing UriRewriter with:
c:\projects\abc\web\yui\menu\assets\skins\sam\menu.css

Code:
$path = 'c:\projects\abc\web\yui\menu\assets\skins\sam\menu.css'
$content = Minify_CSS_UriRewriter::rewrite(
                    file_get_contents($path)
                    ,dirname($path)
                    ,'c:\projects\abc\web'
                    ,array()
            );  

$debugText:
docRoot : sf_root_dir\web 
currentDir : sf_root_dir\web\yui\menu\assets\skins\sam 

file-relative URI : ../../../../assets/skins/sam/sprite.png 
path prepended :
sf_root_dir\web\yui\menu\assets\skins\sam\..\..\..\..\assets\skins\sam\sprite.pn
g 
docroot stripped : 
\yui\menu\assets\skins\sam\..\..\..\..\assets\skins\sam\sprite.png 
traversals removed : /yui/menu/assets/assets/skins/sam/sprite.png 

The correct absolute path should be: /yui/assets/skins/sam/sprite.png

Original comment by laya...@yahoo.com on 5 May 2009 at 2:45

GoogleCodeExporter commented 9 years ago

@laya... Yep, definitely a bug. When I remove the traversals by hand I get the 
right 
URI:
\yui\menu\assets\skins\sam\..\..\..\..\assets\skins\sam\sprite.png
\yui\menu\assets\skins\..\..\..\assets\skins\sam\sprite.png
\yui\menu\assets\..\..\assets\skins\sam\sprite.png
\yui\menu\..\assets\skins\sam\sprite.png
\yui\assets\skins\sam\sprite.png

Original comment by mrclay....@gmail.com on 5 May 2009 at 12:35

Changed state: Accepted

GoogleCodeExporter commented 9 years ago

Fix in R328. Can you verify the UriRewriter.php in that rev works for you? I 
added a 
testcase with your input and it now passes.

Original comment by mrclay....@gmail.com on 5 May 2009 at 1:09

Changed state: Fixed
Added labels: Release-2.1.2

GoogleCodeExporter commented 9 years ago

Fixed!!
YUI has many deeply nested structure, so hopefully this fix will work with even
deeper nesting (more traversals).
Thanks.

Original comment by laya...@yahoo.com on 5 May 2009 at 9:12

GoogleCodeExporter commented 9 years ago

Yay

Original comment by mrclay....@gmail.com on 5 May 2009 at 9:18

Changed state: Verified

GoogleCodeExporter commented 9 years ago

Sort of a general question, but any idea when a tag might be made containing 
this
fix?  Not sure if you have a 2.1.3 release in mind.  Your fix seems to have 
solved
the issues I was having regarding this bug, but I'm hesitant to run off of a 
trunk
checkout in case there is work in progress being done.  No rush, just wondering 
is
all.  Thanks again.

Original comment by bmhar...@gmail.com on 9 Jun 2009 at 9:02

GoogleCodeExporter commented 9 years ago

2.1.3 will be tagged soon. There are bigger changes I want in 2.2.

Original comment by mrclay....@gmail.com on 11 Jun 2009 at 1:14

zhanjh / minify

UriRewriter is confused by many "../" traversals #99