Closed Bug 342662 Opened 14 years ago Closed 14 years ago

Strings are %-escaped in console

Categories

(Toolkit Graveyard :: Error Console, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mozilla, Assigned: Waldo)

References

Details

Attachments

(1 file)

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.
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
Attached patch PatchSplinter Review
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
Closed: 14 years ago
Resolution: --- → FIXED
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?
(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.
I mostly use the Error console to access chrome window and document objects ;-)
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.
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.