tubalmartin / YUI-CSS-compressor-PHP-port

A PHP port of the YUI CSS compressor.
230 stars 34 forks source link

Error with trailing unclosed comments #23

Closed josephscott closed 7 years ago

josephscott commented 9 years ago

I ran into a problem with CSSmin hanging during the minification process. Eventually I tracked it down to a problem with an unclosed comment at the end of the file. It can be reproduced with this CSS:

/* Comment 1 */
/* Comment 2

When I run CSSmin against this it does the infinite loop thing until hitting the max execution time limit in PHP.

Yes the error is that user didn't close the comment, but I think a defensive approach would be to strip the unclosed trailing comment out.

josephscott commented 9 years ago

I'm experimenting with this approach:

diff --git a/cssmin.php b/cssmin.php
index fab0102..ac01811 100644
--- a/cssmin.php
+++ b/cssmin.php
@@ -75,9 +75,22 @@ class CSSmin

         $css = $this->extract_data_urls($css);

+        // Clean up trailing unclosed comments
+        $last_newline = strrpos( rtrim( $css ), "\n" );
+        if ( $last_newline !== false ) {
+            $last_comment_start = strrpos( $css, '/*', $last_newline );
+            if ( $last_comment_start !== false ) {
+                $last_comment_end = strrpos( $css, '*/', $last_comment_start );
+                if ( $last_comment_end === false ) {
+                    $css = substr( $css, 0, $last_newline );
+                }
+            }
+        }
+
         // collect all comment blocks...
         while (($start_index = $this->index_of($css, '/*', $start_index)) >= 0)
             $end_index = $this->index_of($css, '*/', $start_index + 2);
+
             if ($end_index < 0) {
                 $end_index = $length;
             }
@@ -774,4 +787,4 @@ class CSSmin

         return (int) $size;
     }
-}
\ No newline at end of file
+}

tests/mine/unclosed-comment.css

a {
    color: blue;
}
html >/**/ body p {
    color: blue;
    /*a comment right before the IE star hack*/*width:auto;
    height:2%;
}

/* Comment 1 */

/* Comment 2

tests/mine/unclosed-comment.css.min

a{color:blue}html>/**/body p{color:blue;*width:auto;height:2%}
tubalmartin commented 7 years ago

Although I agree I could take a defensive approach to fix that weird case I also think it's a coder's responsibility to have his code linted and checked before doing anything else with it. So, thank you very much for your time and code but that's an issue I won't fix ;)

tubalmartin commented 7 years ago

On v3.0.0, which is about to be released in a few days, the minifier will not hang when it encounters a unclosed comment 👍 It will not strip the comment though because that's a coder's mistake. Thanks for reporting!

futtta commented 7 years ago

so what else will be in 3.0.0 @tubalmartin ? :-)

tubalmartin commented 7 years ago

Fixed in v2.4.8-p10 & v3.0.0