Skip to content

Conversation

@neomoto
Copy link

@neomoto neomoto commented Apr 16, 2013

fixed exception in parsing symbols like em dash or cyrillic letters.
Tests are included

neomoto added 2 commits April 16, 2013 23:36
fixed exception in parsing symbols like em dash or cyrillic letters.
Tests are included
@leafo
Copy link
Owner

leafo commented Apr 16, 2013

Your test compiles fine without applying the patch.

@neomoto
Copy link
Author

neomoto commented Apr 17, 2013

Sorry, I forgot explain when it happens.
If you have string overloading with this settings:
mbstring.internal_encoding = UTF-8
mbstring.func_overload = 2

then you have exception on "parse error: failed at `content: '—';".
Specifically, this row
mbstring.internal_encoding = UTF-8
causes parse error. With only
mbstring.func_overload = 2
parsing goes well.

@robocoder
Copy link

The mbstring.func_overload ini setting can't be changed at run-time; also, I've run into systems where the mbstring extension isn't enabled.

How about prefixing strlen() and substr() calls with '_', and then adding these helper functions?

// mb_orig_strlen() and mb_orig_substr() only exist when mbstring.func_overload & 2 is true and the mbstring extension is loaded
if (function_exists('mb_orig_strlen')) {
    function _strlen($s) {
        return mb_orig_strlen($s);
    }

    function _substr() {
        return call_user_func_array('mb_orig_substr', func_get_args());
    }
} else {
    function _strlen($s) {
        return strlen($s);
    }

    function _substr() {
        return call_user_func_array('substr', func_get_args());
    }
}

@leafo
Copy link
Owner

leafo commented Apr 17, 2013

The language itself doesn't use unicode so the parser doesn't need to be aware of it (no unicode keywords or anything). The buffer can be treated as a string of 8 bit chars and still have unicode characters pass through it fine. I'm guessing the problem is that mbstring.func_overload causes the string functions to work differently than the $buffer[x] character accessor which causes a mismatch and the parser to fail?

Does changing the ini setting with ini_set just for the parse work? If that doesn't work then another option is to replace all instances of $this->buffer[] with substr($this->buffer, ...). There's probably a noticeable performance hit to this change though.

@neomoto
Copy link
Author

neomoto commented Apr 17, 2013

Does changing the ini setting with ini_set just for the parse work?

Yes, you are right.. Changing compile method on this

    public function compile($string, $name = null) {
        $locale = setlocale(LC_NUMERIC, 0);
        setlocale(LC_NUMERIC, "C");

        $internal_encoding = NULL;
        // only if mbstring installed
        if (function_exists('mb_orig_strlen')) {
            $internal_encoding = ini_get('mbstring.internal_encoding');
            if(!is_null($internal_encoding)){
                ini_set('mbstring.internal_encoding', NULL);
            }
        }
        $this->parser = $this->makeParser($name);
        $root = $this->parser->parse($string);

        $this->env = null;
        $this->scope = null;

        $this->formatter = $this->newFormatter();

        if (!empty($this->registeredVars)) {
            $this->injectVariables($this->registeredVars);
        }

        $this->sourceParser = $this->parser; // used for error messages
        $this->compileBlock($root);

        ob_start();
        $this->formatter->block($this->scope);
        $out = ob_get_clean();
        setlocale(LC_NUMERIC, $locale);
        if(!is_null($internal_encoding)){
            ini_set('mbstring.internal_encoding', $internal_encoding);
        }
        return $out;
    }

helps and parsing is going well even if
mbstring.internal_encoding = UTF-8
is set in php.ini or .htaccess

Parse error appears only if you had set
mbstring.internal_encoding UTF-8
@unnamed777
Copy link

Thanks for fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants