search

sensational / sassphp

PHP bindings to libsass - fast, native Sass parsing in PHP!
Other
229 stars 39 forks source link

Make test fail on debian Jessie and php 5.6.7 #19

Closed nicolasbadia closed 9 years ago

nicolasbadia commented 9 years ago

Hi guys,

I'm having trouble to make sassphp work on Debian Jessie with latest PHP. Any idea where the problem could come from ? Here is the bug report I got:

=====================================================================
PHP         : /usr/bin/php
PHP_SAPI    : cli
PHP_VERSION : 5.6.7-1
ZEND_VERSION: 2.6.0
PHP_OS      : Linux - Linux server101 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt9-2 (2015-04-13) x86_64
INI actual  : /tmp/sassphp/tmp-php.ini
More .INIs  :
CWD         : /tmp/sassphp
Extra dirs  :
VALGRIND    : Not used
=====================================================================
TIME START 2015-05-14 02:19:50
=====================================================================
PASS Check for sass presence [tests/available.phpt]
PASS Test Sass is a class [tests/class_exists.phpt]
PASS Test Sass is instantiatable [tests/class_instantiatable.phpt]
PASS correctly handles includePath [tests/handles_include_path.phpt]
*** Error in `/usr/bin/php': free(): invalid next size (fast): 0x0000000001f44240 ***
FAIL correctly handles setting and getting precision [tests/handles_precision.phpt]
PASS correctly handles setting and getting style [tests/handles_style.phpt]
PASS parse_file() parses correct Sass file [tests/parse_file_parses_file.phpt]
*** Error in `/usr/bin/php': free(): invalid next size (fast): 0x0000000002bc8170 ***
FAIL parse() parses correct Sass [tests/parse_parses_correct_sass.phpt]
PASS If there's an error while parsing, throw a SassException [tests/throw_exception.phpt]
=====================================================================
TIME END 2015-05-14 02:19:50

=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped    :    0
Exts tested     :   43
---------------------------------------------------------------------

Number of tests :    9                 9
Tests skipped   :    0 (  0.0%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    2 ( 22.2%) ( 22.2%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :    7 ( 77.8%) ( 77.8%)
---------------------------------------------------------------------
Time taken      :    0 seconds
=====================================================================

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
correctly handles setting and getting precision [tests/handles_precision.phpt]
parse() parses correct Sass [tests/parse_parses_correct_sass.phpt]
=====================================================================
chrinor2002 commented 9 years ago

Hmm. I saw this on ubuntu utopic the other day. My C experience is not good enough to debug where its coming from. Anyone have any ideas?

pilif commented 9 years ago

looks like there's a free happening on not previously allocated memory. I'll have a look.

pilif commented 9 years ago

If I had to guess, I'd say the problem is with line 103 in our sass.c where we do

context->options.include_paths = this->include_paths != NULL ? this->include_paths : "";

and then later in sass_free_storage

    if (obj->include_paths != NULL)
        efree(obj->include_paths);

so if no include_path is set in the PHP object, we're setting the sass include path to a constant which we then proceed to call efree() on.

I'll see whether sass_compile is happy with a NULL include path now (it wasn't back when I initially made this mistake) and then we can get rid of the check in line 103

pilif commented 9 years ago

I investigagted further. The double free happens somewhere inside libsass itself, not related to our invocation. I suspect there's some issue with the old sass_interface.h way of doing things. I'll port our code to the new non-deprecated way of doing it and try again.

nicolasbadia commented 9 years ago

Fixed by https://github.com/sensational/sassphp/pull/21. Thanks for taking care of this so quickly.

pilif commented 9 years ago

reopening because I should probably

the second one is important because two of the commits were security relevant.