angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.81k stars 27.49k forks source link

IMG tag with data-src contains % character fails to compile on IE8+ #8498

Open imcotton opened 10 years ago

imcotton commented 10 years ago

First here is markup code for <img /> tag

  <img data-src="200%x{{ 50 * 2 }}" />

Secondly please checkout the full example runs on JS Bin, under the IE Developer Tools Console will yells invalid argument, and by causing from code in sanitizeUri.js#L66.

Technically speaking, the root cause is that percent character % in URI should be encoded to %25, but there still use cases which that content not intend to be used as a valid URI even it's been put into data-src attribute, please be aware.

Reproducible: always Browsers being tested: IE8 IE9 Operating system: Windows 7

imcotton commented 10 years ago

If helps, this is link to screenshots from BrowserStack

chrisfosterelli commented 10 years ago

I'm pretty sure I know what is causing this:

sanitizeUri makes a call to urlResolve, and the way urlResolve works is to create an arbitrary <a> element and set the URL to the href element of that <a> tag. Then, it has access to information about the URL via properties on the <a> tag provided by the browser. I'm assuming the Angular guys did this to save code and avoid writing their own parsing implementation.

You can see the behaviour where it sets it here: https://github.com/angular/angular.js/blob/36831eccd1da37c089f2141a2c073a6db69f3e1d/src/ng/urlUtils.js#L71

Unfortunately, when IE tries to parse a URL that includes a percentage sign (%) it seems like it tries to parse it as an escaped character. For instance, if you call urlResolve with a URL containing "%20" (escaped space character) everything will work OK, but if you pass in any string that contains a percentage sign and non-valid following escape character ("%x{" in your case) IE throws a vague "Invalid Argument" error.

The issue isn't in Angular, it's in IE. I can confirm that IE10 and IE11 are also affected by this, and it isn't just data-src but any code that calls urlResolve. I first discovered this while trying to make an $http request to a URL that included a "%". An interceptor was supposed to replace the "%", but IE crashes before it gets there.

So, from the perspective of the Angular devs this is kinda an inherit IE bug and not really their problem, but from a developer perspective this took me ALOT of time to track down as the error is extremely vague and it's unclear why it was happening. Not sure what a good solution is, or if this is something that should be fixed in Angular at all. I've seen a few devs get caught by this now. Thoughts?

imcotton commented 10 years ago

@caitp hello...

caitp commented 10 years ago

I'm with @chrisfosterelli --- this is really a bug in IE, (someone should see if there's a bug filed on https://connect.microsoft.com/IE/Feedback --- I checked, but didn't notice any --- and file one if there isn't).

It would be nice to handle this correctly in angular, but the problem is we can't just assume a URL is not "unsafe" just because IE throws when we read urlParsingNode.href, so we'd need some way to determine if there are any percentage signs in the url which are not percent encodings, and replace them with the percent encoding for the percentage character. Such craziness!

I'm not sure we can really do that, though. In this example, we have a "%x", which is clearly not a valid escape character, but if we had "%100", "%10" would be a valid percent encoding, so it's really hard to distinguish between real and not real percent encodings.

I guess we could do a hack to replace any "%" not followed by a digit with "%25", that might be good enough... but I dunno. I am pretty sure this would still end up with the "unsafe:" prefix though, and that's probably not the right thing