Side navigation
#10537 closed bug (invalid)
Opened October 19, 2011 11:49PM UTC
Closed November 01, 2011 11:49PM UTC
Last modified November 03, 2011 08:12PM UTC
jQuery.html(...) doesn't check for invalid characters
Reported by: | error454 | Owned by: | error454 |
---|---|---|---|
Priority: | low | Milestone: | None |
Component: | core | Version: | 1.6.4 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
The following will result in a syntax error. Given some div #mydiv:
var test = "<script>alert(‘boo’);</script>"; $('#mydiv').html(test);
Or to short circuit the test, you end up with the following equivalent call:
var test = "alert(‘boo’);"; jQuery.globalEval(test);
Note that the variable definition is using the glyphed version of the single quotes. The actual problem is the data passed to eval(). Since jQuery is calling eval() with no character validation, this seems like a reasonable bug to report.
Attachments (0)
Change History (9)
Changed October 20, 2011 09:21PM UTC by comment:1
owner: | → error454 |
---|---|
status: | new → pending |
Changed October 24, 2011 06:23PM UTC by comment:2
status: | pending → new |
---|
The expected outcome would be to throw a descriptive error with text indicating that the eval failed along with the data passed into the eval.
Here is a fiddle of what I think the new function should look like:
I tried making a fiddle of the original problem but due to the syntax error, the fiddle didn't bubble up anything useful other than there being a syntax error in their shell.
Changed October 27, 2011 10:16PM UTC by comment:3
status: | new → pending |
---|
What type of character validation would be sufficient in this case? It seems like we would need a Javascript parser to know if there are no syntax errors in the script. Do you see the error message in the Javascript console?
Changed October 31, 2011 05:57PM UTC by comment:4
status: | pending → new |
---|
After thinking on this, I think that character validation is not a good solution. It wouldn't be simple and may be hard to maintain.
If you're using the jQuery html function with an invalid character, the message you get in the javascript console is simply SyntaxError. The message is completely unhelpful, doesn't tell you which character or string was invalid and leaves you wondering whether the problem is jQuery or javascript - in this case it has nothing to do with jQuery besides the fact that jQuery called a javascript function.
Where jQuery can help is by simply wrapping the eval call in a try and reporting errors that come out of it. This immediately tells the programmer that the problem was outside of jQuery and also helps by showing the string sent to eval that caused the problem.
I believe this adds a lot of value to the programmer for very little risk and work on the jQuery side.
In my case I copied a text string from a word document to my IDE and didn't notice that the glyphed characters were there. After about an hour of debugging I finally was able to work up the call stack to the actual problem. 1 simple log statement could save a lot of time for the next guy.
Changed November 01, 2011 11:49PM UTC by comment:5
component: | unfiled → core |
---|---|
priority: | undecided → low |
resolution: | → invalid |
status: | new → closed |
The information provided by default is even more useful. Web Inspector will show a stack trace and firebug shows the code that caused the issue. Good dev tools will do a better job of tracking down the error.
Changed November 03, 2011 07:13PM UTC by comment:6
Replying to [comment:5 timmywil]:
The information provided by default is even more useful. Web Inspector will show a stack trace and firebug shows the code that caused the issue. Good dev tools will do a better job of tracking down the error.
Sure, maybe for the desktop, but there aren't such tools for debugging mobile webkit. When debugging on mobile you rely completely on getting enough console log details to be able to repro the problem on the desktop.
Not everyone has the luxury of being able to test all customer generated content in a lab environment across the multitude of webkit implementations that handset vendors release.
I reported this bug to try and aid other developers using jQuery, I'm disappointed that this is being closed as invalid. To the next android webkit developer, sorry, I tried.
Changed November 03, 2011 07:25PM UTC by comment:7
The fact is try/catch's make debugging worse. Instead of lobbying for jQuery to integrate universal debugging tools so that android webkit will have a slightly more descriptive error message, persuade android webkit to make better debugging tools.
Changed November 03, 2011 07:43PM UTC by comment:8
Also, I'd strongly recommend designing your mobile application so you can do as much debugging as possible on a desktop client. The problem you encountered had nothing to do with mobile-specific issues, and could have been debugged elsewhere.
If you need mobile client-error telemetry on an ongoing basis you might want to look into something like DamnIT or Runtime Intelligence. Those are certainly outside the scope of jQuery though, and require an application that is designed with error handling in mind.
Changed November 03, 2011 08:12PM UTC by comment:9
I don't disagree with what you guys are saying, things are just more complicated in reality. The problem didn't have anything to do with mobile but that's the domain where I had to begin tracking the issue down. The issue was debugged elsewhere once I had enough details to repro.
I've looked extensively into Loggly and DamnIT, I'm just not permitted to use them in production code.
I lobby google frequently, persuading them to do anything is an exercise in fantasy. I'll continue to file bugs that I think are relevant against jQuery, it doesn't mean I have to like the resolution.
The script does seem to have a syntax error, so can you explain your desired outcome? Even better, can you provide an example at jsFiddle.com?