Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

make sure we have the same encoding in the string and regex #2650

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

dmathieu commented Aug 23, 2011

This fixes the failing actionpack test.

Contributor

josevalim commented Aug 23, 2011

Hrm, I am not sure force_encoding is the proper fix here. If it is not a UTF-8 string, it is going to fail badly and we cannot assume that. /cc @tenderlove @wycats

Owner

pixeltrix commented Aug 23, 2011

Doesn't it just need a /u on the regexp?

Contributor

dmathieu commented Aug 23, 2011

No, /u is not enough when the string is not UTF-8. I get the following error :

Encoding::CompatibilityError: incompatible encoding regexp match (UTF-8 regexp with ASCII-8BIT string)
Contributor

dmathieu commented Aug 23, 2011

An other solution would be to run the last JS_ESCAPE_MAP entry only when the string is UTF-8.
Something like :

result = javascript.gsub(/(\\|<\/|\r\n|\342\200\250|[\n\r"'])/u) do |match|
    if JS_ESCAPE_MAP.has_key?(match)
        JS_ESCAPE_MAP[match]
    elsif match.is_utf8? && JS_UTF8_ESCAPE_MAP.has_key?(match)
        JS_UTF8_ESCAPE_MAP[match]
    else
        match
    end
end
Contributor

dmathieu commented Aug 23, 2011

See #2659, which closes the issue.

@dmathieu dmathieu closed this Aug 23, 2011

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