Error with trailing unclosed comments #23

Closed
josephscott opened this Issue Mar 18, 2015 · 5 comments

Comments

Projects
None yet
3 participants

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.

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%}
Owner

tubalmartin commented Mar 26, 2017

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 ;)

Owner

tubalmartin commented Mar 31, 2017

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 Mar 31, 2017

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

Owner

tubalmartin commented Apr 4, 2017

Fixed in v2.4.8-p10 & v3.0.0

Furniel added a commit to Furniel/W3-Total-Cache that referenced this issue May 4, 2017

Merge branch 'YUI-CSS-compressor-PHP-3x' into Minify_2.3.1
YUI-CSS-compressor-PHP-3x official changelog:

### v3.1.2 17 Apr 2017

* Improved compression of long named colors: now all long named colors get compressed to their shorter HEX counterpart.
* Fixes cases such as [#39](tubalmartin/YUI-CSS-compressor-PHP-port#39)
* Huge performance improvement after code profiling. See table below for results when running the whole test suite:

PHP version used: 5.3.29

| chunkLength | v3.1.1 | v3.1.2 |
| --- | --- | --- |
| 100 | 6.8s | 2.6s |
| 1000 | 5.3s | 2s |
| 2000 | 5.2s | 1.95s |
| 5000 | 5.1s | 1.9s |

PHP version used: 7.0.8

| chunkLength | v3.1.1 | v3.1.2 |
| --- | --- | --- |
| 100 | 2s | 0.72s |
| 1000 | 1s | 0.37s |
| 2000 | 0.8s | 0.33s |
| 5000 | 0.7s | 0.3s |

### v3.1.1 11 Apr 2017

* Regexes improved.
* Small performance improvements.
* Blocks such as `@media` blocks with empty rules are removed too.
* Quoted unquotable attribute selectors get unquoted now i.e. from `col[class*="col-"]` to `col[class*=col-]`. Covers most common cases. Safe approach.

### v3.1.0 9 Apr 2017

* Code deeply analyzed. Some areas rewritten from the ground up with maximum performance in mind. No change in compressor behavior.
* Fixed some hidden bugs discovered along the way that affected performance negatively.
* IE5/Mac comment hack removed from minifier logic. Those comments will no longer be preserved.
* The table below displays the performance optimization done in this version in comparison with the previous one running the whole test suite:

PHP version used: 5.3.29

| chunkLength | v3.0.0 | v3.1.0 |
| --- | --- | --- |
| 100 | 38s | 6.9s |
| 1000 | 8.5s | 5.4s |
| 2000 | 7.3s | 5.3s |
| 5000 | 5.8s | 5.2s |

PHP version used: 7.0.8

| chunkLength | v3.0.0 | v3.1.0 |
| --- | --- | --- |
| 100 | 22.8s | 2.1s |
| 1000 | 2.9s | 1.1s |
| 2000 | 2s | 0.9s |
| 5000 | 1.3s | 0.8s |

### v3.0.0 4 Apr 2017

* New API compliant with PSR-1, PSR-2 & PSR-4. PHP 5.3.2+ required. I think it was time!
* Many tests added, strengthened and fixed. Big, real life, stylesheets included such as Bootstrap or Foundation.
* Fixed some critical and minor issues, such as:
   * Chunking system breaking some stylesheets (broken at rules block) or leaving some bits off.
   * Backreferences in replacement strings breaking stylesheets.
   * [#23](tubalmartin/YUI-CSS-compressor-PHP-port#23)
   * Others...
* Color compression improved. Now all named colors are supported i.e. from `white` to `#fff`.
* Shortening zero values is back but in a safe manner, shortening values assigned to "safe" properties only i.e. from `margin: 1px 0.0em 0rem 0%` to `margin:1px 0 0`. Check the code to see the list of "safe" properties.
* `padding` and `margin` properties are shortened to the bare minimum i.e. from `margin: 3px 2.1em 3px 2.1em` => `margin:3px 2.1em`
* Upgrading to v3 is strongly recommended for users enjoying PHP 5.3.2+.

### v2.4.8-p10 4 Apr 2017

* This is the last v2 release. v3 onwards will only support PHP 5.3.2+.
* This patch has all improvements and fixes v3.0.0 has. See v3.0.0 notes for further info (no API change in this version of course).
* Updating to this patch is strongly recommended for users stuck with PHP versions lower than PHP 5.3.

### v2.4.8-p9 28 Mar 2017

* Rolling back property declaration with scalar expressions (>= PHP 5.6) introduced in v2.4.8-p8 to support PHP 5.0. No change in compressor behavior.

### v2.4.8-p8 27 Mar 2017

* Fixed issue [#18](tubalmartin/YUI-CSS-compressor-PHP-port#18)
* Added `set_chunk_length` method.
* `bold` & `normal` values get compressed to `700` & `400` respectively for `font-weight` property.
* GUI updated.
* FineDiff library loaded through Composer.
* README updated.

### v2.4.8-p7 26 Mar 2017

* Fixed many issues [#20](tubalmartin/YUI-CSS-compressor-PHP-port#20), [#22](tubalmartin/YUI-CSS-compressor-PHP-port#22), [#24](tubalmartin/YUI-CSS-compressor-PHP-port#24), [#25](tubalmartin/YUI-CSS-compressor-PHP-port#25), [#26](tubalmartin/YUI-CSS-compressor-PHP-port#26) reported by contributors and others that I'm sure haven't been reported, at least yet. Sorry for the long delay guys.
* This release is all about stability and reliability and as such I've had to take some controversial decisions such as:
   * Not minifying `none` property value to `0` because in some subtle scenarios the resulting output may render some styles differently.
   * Not removing units from zero length values because in many cases the output will break the intended behavior. Patching every single case after someone finds a new breaking case is not good IMHO taking into account CSS is a live spec and browsers differ in some cases.
* Hope you agree with me removing those conflicting parts. Enjoy this release :)

### v2.4.8-p6 21 Mar 2017

* Fixed PHP CLI issues. See [#36](tubalmartin/YUI-CSS-compressor-PHP-port#36)

### v2.4.8-p5 27 Feb 2017

* Fixed PHP 7 issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment