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

feat(sce): trust resourceUrls where url is in a $cache #4879

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Nov 11, 2013

Enables sce.getTrustedResourceUrl and sce.parseAsResourceUrl to optionally look up urls in a $cacheFactory-generated cache, primarily the $templateCache.

I'd like to discuss a few things here:

  1. Are things in $cacheFactory caches really trusted? In my mind, they should be. But there may be things I'm missing.
  2. How useful would this be for non-template resources? Would it be possible to make it more useful?
  3. Is there something I'm missing where perhaps this isn't actually necessary at all? It seemed to be an issue on plnkr.co, although I haven't tested it outside of here just yet.

If it gets merged, it would be a simple thing to modify ng-include and other similar $sce-using core directives to work with it. Or perhaps it might make more sense to enable this as the default behaviour. I'm interested in finding out.

Enables sce.getTrustedResourceUrl and sce.parseAsResourceUrl to
optionally look up urls in a $cacheFactory-generated cache, primarily
the $templateCache.
if (cacheFactory && (cache === true || typeof cache === 'string')) {
cache = cacheFactory.get((cache === true && 'templates') || cache);
if (cache && typeof cache.get(parsedUrl.href) === 'string') {
return true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resource url is assumed to be trustworthy if we've already cached it somewhere. (is === 'string' the best way to do this?)

@shahata
Copy link
Contributor

shahata commented Feb 7, 2014

I was going to create a PR for this myself and found this one while browsing through existing PR's. I have to say though, that this is not the way I thought that this issue needs to be handled. I don't like it that when I invoke sce.getTrustedResourceUrl and sce.parseAsResourceUrl with a cache param I essentially 'promise' that later when I invoke $http.get I will use the same cache param.

When I thought about how to fix this issue, my idea was actually to move the sce.getTrustedResourceUrl invocations into a new method in $http (or tell $http through the config object that the url needs to be trusted) and skip the $sce invocation in case $http knows for sure that it is going to use the reply from the cache.

@shahata
Copy link
Contributor

shahata commented Feb 7, 2014

BTW, the reason this issue was bothering me is that we use a 'base' tag in our html's that point to our CDN in order to make it easier to fetch static resources with relative url's even though the page is served from somewhere else completely. We normally load html partials (ng-include, ng-view, etc.) from the same origin that we load the page from, so no issue there. BUT, many libraries actively put things in the $templateCache and almost always, they use a relative URL. (which actually point to our CDN because of the 'base' tag and get blocked by $sce) So I am stuck in a situation where although I know for sure that there is not going to be any http request, the sce blocks it because the URL is not trusted.

@caitp
Copy link
Contributor Author

caitp commented Oct 2, 2014

@shahata can you open an issue regarding your use case? I think we're just going to close this PR, it's bitrotten anyways

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants