Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

angular-sanitize.js remove internal links #3748

Closed
janbaer opened this issue Aug 26, 2013 · 6 comments
Closed

angular-sanitize.js remove internal links #3748

janbaer opened this issue Aug 26, 2013 · 6 comments
Assignees

Comments

@janbaer
Copy link

janbaer commented Aug 26, 2013

I've written an wiki application that is showing the converted markdown as html in a div element that is using the ng-bind-html directive.

I have in my html some internal links in this format

<a href='/page1.md'>Page1</a>

My html will be processed by the angular-sanitize.js. And here is my problem, the final html doesn't contains my link. I see only the text Page1.

I've looked into the code of the angular-sanitize.js file and I've found the regex that is responsible to detect any links:

URI_REGEXP = /^((ftp|https?):\/\/|mailto:|tel:|#)/i,

After I've changed the regex to this, the link wasn't removed.

URI_REGEXP = /^((ftp|https?):\/\/|mailto:|tel:|#|\/)/i,

Could you fix this please in the original version of the angular-sanitize.js? Thanks!

@drdaeman
Copy link

Just had the same issue and I must admit I don't get the whole point of sanitizing href attributes this way.

Can anybody knowledgeable, please, explain what's the whole point of uriAttrs? What're the potentially unwanted cases they're protecting from? I'd understand if there was a some sort of user-configurable callback that'd let users filter out unwanted destinations, but current URI_REGEXP looks completely pointless to me.

@jacobsvante
Copy link

Anyone?

@brandonaaskov
Copy link

I'm weighing in on this too - seems silly that we can't use relative links in an application if we also want to use the $sanitize service and it's related directives.
screenshot 2013-11-13 17 11 54

@janbaer
Copy link
Author

janbaer commented Nov 14, 2013

I've created this pull request that fixes the problem. I's only one char more in a reqular expression more.

#4736

@ghost ghost assigned tbosch Nov 21, 2013
@mhevery
Copy link
Contributor

mhevery commented Nov 21, 2013

The reason why it works this way is that a link could be which would cause script injection. Unfortunately it is not easy to determine if a link is prefixed with javascript: since there are a lot of ways in which the javascript keyword can be encoded/escaped. So the safest way is to just assuming that the link must be fully qualified ie start with http and so on.

I find this a slippery slope by adding / to the list of things which it can start with, since you could also start with .. and ./ and so on.

I think a better way to fix this is to let the browser decode the URL and assert that it starts with http, just like we do here: https://github.com/angular/angular.js/blob/master/src/ng/compile.js#L718 This would solve all of the use cases in a trusted way.

@tbosch
Copy link
Contributor

tbosch commented Nov 21, 2013

@mhevery Had the same thought yesterday, but added a message to the PR (#4736 (comment)). Sorry for the double work...
The idea is that $sanitize uses the same logic as $compile does, and also the same whitelist regex.

tbosch added a commit to tbosch/angular.js that referenced this issue Nov 26, 2013
`$sanitize` now uses the same mechanism as `$compile` to validate uris.
By this, the validation in `$sanitize` is more general and can be
configured in the same way as the one in `$compile`.

Changes
- Creates the new private service `$$sanitizeUri`.
- Moves related specs from `compileSpec.js` into `sanitizeUriSpec.js`.
- Refactors the `linky` filter to be less dependent on `$sanitize`
  internal functions.

Fixes angular#3748.
@tbosch tbosch closed this as completed in 3335234 Nov 26, 2013
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
`$sanitize` now uses the same mechanism as `$compile` to validate uris.
By this, the validation in `$sanitize` is more general and can be
configured in the same way as the one in `$compile`.

Changes
- Creates the new private service `$$sanitizeUri`.
- Moves related specs from `compileSpec.js` into `sanitizeUriSpec.js`.
- Refactors the `linky` filter to be less dependent on `$sanitize`
  internal functions.

Fixes angular#3748.
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
`$sanitize` now uses the same mechanism as `$compile` to validate uris.
By this, the validation in `$sanitize` is more general and can be
configured in the same way as the one in `$compile`.

Changes
- Creates the new private service `$$sanitizeUri`.
- Moves related specs from `compileSpec.js` into `sanitizeUriSpec.js`.
- Refactors the `linky` filter to be less dependent on `$sanitize`
  internal functions.

Fixes angular#3748.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.