Bad escape sequence \b

[Andrea Ercolino]

​https://dev.ckeditor.com/browser/CKEditor/trunk/ckeditor_php5.php#L559 ​https://dev.ckeditor.com/browser/CKEditor/trunk/ckeditor_php4.php#L569

What's the \b escape sequence in above lines? Zend Studio complains with a warning, and I'm quite sure it's right.

I'd suppress the substitution, what do you think?

[Andrea Ercolino]

PHP doc: ​http://php.net/manual/en/language.types.string.php#language.types.string.syntax.double

JavaScript doc: ​https://developer.mozilla.org/en/JavaScript/Guide/Values,_Variables,_and_Literals#Using_Special_Characters_in_Strings

If the intended use was to provide escaping for \b in a JavaScript string, in PHP that \b should be coded '\b' or "
b", not "\b".

In that case, all the source substitutions in ckeditor_php5.php#L559 and ckeditor_php4.php#L569 should be coded the same as \b, otherwise the program is syntactically correct but semantically wrong. In fact, for example, it's purely coincidental that in PHP and JavaScript a new line is represented by the same \n expression.

So,

$jsonReplaces = array(array("\\", "/", "\n", "\t", "\r", "\b", "\f", '"'), array('\\\\', '\\/', '\\n', '\\t', '\\r', '\\b', '\\f', '\"'));

should become

$jsonReplaces = array(array('\\', '/', '\n', '\t', '\r', '\b', '\f', '"'), array('\\\\', '\\/', '\\n', '\\t', '\\r', '\\b', '\\f', '\"'));

I don't know much about ckeditor and its internals, nor about how this jsEncode is used, but it seems a bit buggy. (what is the static keyword for? is the forward slash to be escaped? ...)

[Wiktor Walc]

The static keyword was fine, it was there just to speed up things as static variables are initialized only once. The backspace key was indeed handled in a wrong way.

[Andrea Ercolino]

The conversion of a string S in PHP to a string T in JSON is more like this:

1. X = convert S to UTF-8 (if needed)
2. Y = escape with a backslash any double quote, backslash or control character in X
3. Z = replace any occurrence in Y of the regex

   @</([A-Za-z]+)@
   

   with

   <\/\1
   

4. T = wrap Z into double quotes

About the solidus issue see ​http://noteslog.com/post/the-solidus-issue/

About the static, I see that the patch tries to make it useful, but it looks like a joke.

[Wiktor Walc]

The first patch was wrong as we can simply use there a hex value for the backspace character to avoid the function call (I have no idea why I've missed it).

@aercolino:

1) this should be done by the developer as we have no idea which encoding is used
2-4) Regular expressions are slow, it is much faster to escape each occurrence of /, just like it was suggested by the RFC4627 you mentioned. Note that because the string is outputed with double quotes it makes no difference if at the end there is http://example.com or http:\/\/www.example.com

I left the static keyword, it makes no sense to remove it because it speeds up that part of code.

[Andrea Ercolino]

Look at this conversion example of a JavaScript regular expression in S:

S = /\/\w+/ 
T = \/\\\/\\w+\/
T = /\\/\\w+/

I think that it makes quite a difference to have slashes escaped or not.

About the static, I know that it can be difficult to change our mind, but believe me, it looks like junior code. There is no reason for using a static there.

And the RFC says that every control character must be escaped. Why do you only treat the special cases there?

[Wiktor Walc]

If you need to pass a regular expression or any other JavaScript code, use "@@" character to mark such code and it will remain untouched:

$CKEditor->config['foo'] = "@@/\/\w+/";
$CKEditor->config['bar'] = "@@2+2";

Please provide some real sample where escaping slashes is causing a problem with creating a CKEditor instance and/or the configuration option passed to CKEditor is misunderstood by the editor.

Regarding static - there are as many ways of creating the same code as developers out there, so let's just focus on the functionality.

Good point with control characters.

[Wiktor Walc]

Fixed with [6593].