Strings are %-escaped in console

RESOLVED FIXED

Status

Toolkit Graveyard
Error Console
RESOLVED FIXED
11 years ago
a year ago

People

(Reporter: Neil Fraser, Assigned: Waldo)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
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

11 years ago
The simple fix would be to encodeURI the code before setting it as iframe source.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Mac OS X 10.3 → All
Hardware: Macintosh → All
Duplicate of this bug: 366273
Created attachment 250907 [details] [diff] [review]
Patch
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Attachment #250907 - Flags: review?(gavin.sharp)
Attachment #250907 - Flags: review?(gavin.sharp) → review+
Fixed on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Comment 5

11 years ago
For reference: Console² uses .replace(/%/g, "%25") instead of encodeURI to keep the source URLs of evaluated JavaScript slightly more readable.
What do you mean by "the source URLs of evaluated JavaScript"? Where are these exposed to the user?

Comment 7

11 years ago
(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.
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?
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.
Exceptions in code run in a sandbox have console.js as the filename, so it isn't really suitable.

Comment 11

11 years ago
I mostly use the Error console to access chrome window and document objects ;-)

Comment 12

11 years ago
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.
(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?
(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.

Updated

10 years ago
Depends on: 407751
Product: Firefox → Toolkit
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.