Opened 7 years ago

Closed 6 years ago

#6913 closed Bug (fixed)

Bad escape sequence \b

Reported by: aercolino Owned by: wwalc
Priority: Normal Milestone: CKEditor 3.5.3
Component: Server : PHP Version: 3.1
Keywords: Cc:


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?

Attachments (3)

6913.patch (1.8 KB) - added by wwalc 6 years ago.
6913_2.patch (1.4 KB) - added by wwalc 6 years ago.
6193_3.patch (5.7 KB) - added by wwalc 6 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 7 years ago by aercolino

PHP doc:

JavaScript doc:,_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.


$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? ...)

comment:2 Changed 7 years ago by Saare

  • Component changed from General to Server : PHP
  • Keywords HasPatch added
  • Status changed from new to confirmed
  • Version changed from 3.5.1 (SVN - trunk) to 3.1

comment:3 Changed 6 years ago by wwalc

  • Milestone set to CKEditor 3.5.2

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.

Changed 6 years ago by wwalc

comment:4 Changed 6 years ago by wwalc

  • Owner set to wwalc
  • Status changed from confirmed to review

comment:5 Changed 6 years ago by aercolino

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
  4. T = wrap Z into double quotes

About the solidus issue see

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

Changed 6 years ago by wwalc

comment:6 Changed 6 years ago by wwalc

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


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 or http:\/\/

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

comment:7 Changed 6 years ago by aercolino

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?

comment:8 Changed 6 years ago by wwalc

  • Status changed from review to assigned

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.

Changed 6 years ago by wwalc

comment:9 Changed 6 years ago by wwalc

  • Keywords HasPatch removed
  • Status changed from assigned to review

comment:10 Changed 6 years ago by mkesicki

  • Status changed from review to review_passed

comment:11 Changed 6 years ago by wwalc

  • Resolution set to fixed
  • Status changed from review_passed to closed

Fixed with [6593].

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy