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

feat($templateRequest): trust url if it is already in cache#9745

Closed
shahata wants to merge 2 commits into
angular:masterfrom
shahata:sce-templaterequest
Closed

feat($templateRequest): trust url if it is already in cache#9745
shahata wants to merge 2 commits into
angular:masterfrom
shahata:sce-templaterequest

Conversation

@shahata

@shahata shahata commented Oct 22, 2014

Copy link
Copy Markdown
Contributor

This is a rehash of #4879. I've encountered this issue a long time ago and it is no longer relevant for me, but maybe it will help someone else... This bothered me when I used the base tag in some project to point to a domain different than the one the page was served from - it meant that any template that was preloaded using something like <script type="text/ng-template" id="/tpl.html"> was blocked by $sce when I tried to load it since the domain in the base tag was not trusted by default.

So I'm thinking that it doesn't make much sense to block something that is already in the cache, but there was a test in$compile which explicitly tested that even cache will be blocked (which I had to remove), so I'm probably missing something.

Anyway, even if this is not really interesting, I think that the refactor in the first two commits might be relevant anyway....

@shahata

shahata commented Nov 21, 2014

Copy link
Copy Markdown
Contributor Author

Ay thoughts about this one?

@googlebot

Copy link
Copy Markdown

Thanks for your pull request.

It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits.

Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name.

@caitp

caitp commented Nov 21, 2014

Copy link
Copy Markdown
Contributor

I like it... Igor's assigned (I think we discussed this a while ago, he liked your version more than mine). But I am not sure they'll get to that this week, because of thanksgiving + other stuff. Maybe the bugmail will ping him to take a look though

@googlebot

Copy link
Copy Markdown

CLAs look good, thanks!

@shahata

shahata commented Dec 5, 2014

Copy link
Copy Markdown
Contributor Author

rebased...

@petebacondarwin

Copy link
Copy Markdown
Contributor

I like this. I can't think of how this could become a security issue.

@petebacondarwin

Copy link
Copy Markdown
Contributor

Oh, this was already landed in 1.4.x by #12240
We are not adding stuff like this to 1.3.x

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.

6 participants