Closed Bug 1467093 Opened 2 years ago Closed 2 years ago

Update the content type of the channel used within nsHTMLDocument::Open()

Categories

(Core :: DOM: Security, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: ckerschb, Assigned: ckerschb)

References

Details

(Whiteboard: [domsecurity-active])

Attachments

(1 file, 1 obsolete file)

When working on Bug 965637 I realized that the channel used within  nsHTMLDocument::Open() uses TYPE_OTHER, but in fact that is a document load, hence we should use TYPE_DOC or TYPE_SUBDOC to accurately reflect that state within the loadinfo.
Assignee: nobody → ckerschb
Blocks: 965637
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [domsecurity-active]
Hey Smaug, I would like to have the channel within nsHTMLDocument::Open() use the right content policy type, hence re-using the content type of the current document that gets replaced.

Only problem, GetChannel() might return null and in such a case we can't query the accurate content policy type from the loadinfo. I guess using the accurate type whenever possible is already a win, but ultimately I would like to never pass TYPE_OTHER. If you have a suggestions how we could figure the type in case GetChannel returns null, then I am more than happy to incorporate such a change - thanks!
Attachment #8983731 - Flags: review?(bugs)
Shouldn't top level content page use TYPE_DOCUMENT and (i)frames TYPE_(I)FRAME.
And one could take a look at nsDocShell code to find the right type, no?
Comment on attachment 8983731 [details] [diff] [review]
bug_1467093_update_type_within_doc_open.patch

I think we should just always pass some document related policy type.
Attachment #8983731 - Flags: review?(bugs) → review-
(In reply to Olli Pettay [:smaug] from comment #2)
> Shouldn't top level content page use TYPE_DOCUMENT and (i)frames
> TYPE_(I)FRAME.
> And one could take a look at nsDocShell code to find the right type, no?

Right, I looked at nsDocshell initially but it's quite so much code to figure out the right type, hence I thought simply using the content type of the current document's channel might be an easier approach.

Anyway, within this patch I replicated the code from nsDocshell [1] modulo the isTargetTopLevelDocShell bits, because I think those are not needed, or do we need those as well? 

[1] https://dxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#9361-9375
Attachment #8983731 - Attachment is obsolete: true
Attachment #8984109 - Flags: review?(bugs)
Comment on attachment 8984109 [details] [diff] [review]
bug_1467093_update_type_within_doc_open.patch

>+  // We are using the same technique as withn nsDocShell to figure
"as in" or "as with" or something

>+  // out the content policy type. If there is no same type parent,
>+  // we know we are loading a new top level document.
>+  nsContentPolicyType policyType;
>+  if (!parent) {
>+    policyType = nsIContentPolicy::TYPE_DOCUMENT;
>+  }
>+  else {
} else {

>+    nsCOMPtr<Element> requestingElement;
>+    nsCOMPtr<nsPIDOMWindowInner> window = GetInnerWindow();
These don't need to be nsCOMPtrs. Raw pointers are ok. Initialize requestionElement then to nullptr;

>+    if (window) {
>+      nsCOMPtr<nsPIDOMWindowOuter> outer =
>+        nsPIDOMWindowOuter::GetFromCurrentInner(window);
no need for nsCOMPtr

>+      if (outer) {
>+        RefPtr<nsGlobalWindowOuter> win = nsGlobalWindowOuter::Cast(outer);
no need for RefPtr
Attachment #8984109 - Flags: review?(bugs) → review+
Pushed by mozilla@christophkerschbaumer.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c5e6b5bd946
Update the content type of the channel used within nsHTMLDocument::Open(). r=smaug
https://hg.mozilla.org/mozilla-central/rev/7c5e6b5bd946
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.