Last Comment Bug 342662 - Strings are %-escaped in console
: Strings are %-escaped in console
Status: RESOLVED FIXED
:
Product: Toolkit Graveyard
Classification: Graveyard
Component: Error Console (show other bugs)
: unspecified
: All All
-- normal (vote)
: ---
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
:
Mentors:
: 366273 (view as bug list)
Depends on: 407751
Blocks:
  Show dependency treegraph
 
Reported: 2006-06-24 23:39 PDT by Neil Fraser
Modified: 2016-06-29 11:02 PDT (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (974 bytes, patch)
2007-01-08 15:36 PST, Jeff Walden [:Waldo] (remove +bmo to email)
gavin.sharp: review+
Details | Diff | Splinter Review

Description User image Neil Fraser 2006-06-24 23:39:24 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4

Literal strings typed into the JavaScript evaluation box are interpreted as %-escaped.

Reproducible: Always

Steps to Reproduce:
1. Open JavaScript Console, type the following:
2. alert("a%2Cb")  [Press Enter]
  or 
3. alert("a%2Cb".length)  [Press Enter]

Actual Results:  
"a,b" or 3

Expected Results:  
"a%2Cb" or 5
This is what happens if the above lines are executed from a <SCRIPT> tag.
Comment 1 User image Simon Bünzli 2006-06-25 01:19:12 PDT
The simple fix would be to encodeURI the code before setting it as iframe source.
Comment 2 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-01-08 15:23:09 PST
*** Bug 366273 has been marked as a duplicate of this bug. ***
Comment 3 User image Jeff Walden [:Waldo] (remove +bmo to email) 2007-01-08 15:36:52 PST
Created attachment 250907 [details] [diff] [review]
Patch
Comment 4 User image Jeff Walden [:Waldo] (remove +bmo to email) 2007-01-08 16:22:34 PST
Fixed on trunk.
Comment 5 User image Simon Bünzli 2007-01-08 22:43:57 PST
For reference: Console² uses .replace(/%/g, "%25") instead of encodeURI to keep the source URLs of evaluated JavaScript slightly more readable.
Comment 6 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-01-08 22:50:48 PST
What do you mean by "the source URLs of evaluated JavaScript"? Where are these exposed to the user?
Comment 7 User image Simon Bünzli 2007-01-08 22:57:48 PST
(In reply to comment #6)
What you enter for evaluation gets converted into a javascript: URL which is displayed again should your code contain errors or produce warnings. Try entering something like |[""]..length| to see this.
Comment 8 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-01-09 00:08:26 PST
Ah, yeah, I see what you mean. That is rather suboptimal, I didn't realize that the javascript: URL was shown in the exceptions/warnings. Your approach is certainly better in that regard, I think I prefer it. Jeff, what do you think?
Comment 9 User image Jeff Walden [:Waldo] (remove +bmo to email) 2007-01-09 17:12:43 PST
If we could be sure that any Unicode characters in the evaluated string would be processed properly without being escaped, then yes, I'd be fine with it.  However, I'm not sure we can guarantee that, and given the layers it must pass through, I'm not sure I could prove to myself one way or the other that Unicode would or wouldn't be handled correctly.

Is there any reason we shouldn't remove the iframe and just use Components.utils.evalInSandbox to avoid this whole problem?  The only loss I see is maybe no access to window/document objects, but those were at best unintentionally exposed here.
Comment 10 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-01-09 17:20:08 PST
Exceptions in code run in a sandbox have console.js as the filename, so it isn't really suitable.
Comment 11 User image neil@parkwaycc.co.uk 2007-01-10 01:06:41 PST
I mostly use the Error console to access chrome window and document objects ;-)
Comment 12 User image Simon Bünzli 2007-01-10 07:01:20 PST
If you want to be sure about non-ASCII Unicode characters, what about .replace(/[^\x20-\x24\x26-\x7F]+/g, encodeURI) as a compromise. This encodes everything except printable 7-bit-ASCII characters (% being the exception to the exception).

OTOH I haven't seen any bug report to that end so far and we haven't encoded any (Unicode) characters since day 0, so we might just as well escape only that one character (which shouldn't make much of a difference) and wait for further bug reports such as this one about other characters.
Comment 13 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-02-01 13:02:40 PST
(In reply to comment #12)
> If you want to be sure about non-ASCII Unicode characters, what about
> .replace(/[^\x20-\x24\x26-\x7F]+/g, encodeURI) as a compromise. This encodes
> everything except printable 7-bit-ASCII characters (% being the exception to
> the exception).
> 
> OTOH I haven't seen any bug report to that end so far and we haven't encoded
> any (Unicode) characters since day 0, so we might just as well escape only that
> one character (which shouldn't make much of a difference) and wait for further
> bug reports such as this one about other characters.

Can you file a new bug about this, so this current problem/proposed change doesn't get lost?
Comment 14 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-05-11 09:09:30 PDT
(In reply to comment #13)
> Can you file a new bug about this, so this current problem/proposed change
> doesn't get lost?


Bug 369097 was filed.

Note You need to log in before you can comment on or make changes to this bug.