Closed
Bug 323924
Opened 19 years ago
Closed 18 years ago
Call CheckLoadURI before issuing pings
Categories
(Core :: DOM: Navigation, defect, P2)
Core
DOM: Navigation
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: darin.moz, Assigned: sicking)
References
Details
(Whiteboard: [blocking1.9a1+] if a ping is on by default)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
Call CheckLoadURI before issuing pings We should restrict pings as we restrict redirects.
Reporter | ||
Updated•19 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.9alpha
![]() |
||
Updated•19 years ago
|
Flags: blocking1.9a1+
Comment 1•19 years ago
|
||
I think it should also check with content policy (::ShouldProcess?), so content policy can selectively veto the ping.
Reporter | ||
Comment 2•19 years ago
|
||
Yeah, dveditz made the same comment to me over IM. I plan to include that in the forthcoming patch.
Reporter | ||
Comment 4•19 years ago
|
||
My patch in bug 324642 limits pings to HTTP and HTTPS only, so that should "fix" this bug as well.
Depends on: 324642
![]() |
||
Comment 6•19 years ago
|
||
Actually, we should still call CheckLoadURI... There's no reason we won't fix things so some sites can't link to others (eg via capabilities) at some point.
Reporter | ||
Updated•19 years ago
|
Priority: -- → P2
Updated•18 years ago
|
Flags: blocking1.9+
Flags: blocking1.9a1+
Whiteboard: [blocking1.9a1+] if a ping is on by default
Assignee: darin.moz → bugmail
Status: ASSIGNED → NEW
Assignee | ||
Comment 8•18 years ago
|
||
This should do it
Attachment #246722 -
Flags: superreview?(jst)
Attachment #246722 -
Flags: review?(jst)
Assignee | ||
Comment 9•18 years ago
|
||
jst says it's ok to depend on the scriptsecuritymanager to be available.
Attachment #246722 -
Attachment is obsolete: true
Attachment #246724 -
Flags: superreview?(jst)
Attachment #246724 -
Flags: review?(jst)
Attachment #246722 -
Flags: superreview?(jst)
Attachment #246722 -
Flags: review?(jst)
Comment 10•18 years ago
|
||
Comment on attachment 246722 [details] [diff] [review] Patch to fix r+sr=jst
Attachment #246722 -
Flags: superreview+
Attachment #246722 -
Flags: review+
Updated•18 years ago
|
Attachment #246724 -
Flags: superreview?(jst)
Attachment #246724 -
Flags: superreview+
Attachment #246724 -
Flags: review?(jst)
Attachment #246724 -
Flags: review+
Updated•18 years ago
|
Attachment #246722 -
Flags: superreview+
Attachment #246722 -
Flags: review+
Assignee | ||
Comment 11•18 years ago
|
||
Checked in
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•18 years ago
|
||
Ugh, missed the comments at the top of this bug, there's some more work to be done here.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 13•18 years ago
|
||
This one adds all needed checks both on initial load and on redirect.
Attachment #246724 -
Attachment is obsolete: true
Attachment #246862 -
Flags: superreview?(jst)
Attachment #246862 -
Flags: review?(jst)
Comment 14•18 years ago
|
||
Comment on attachment 246862 [details] [diff] [review] This one's better >Index: base/nsWebShell.cpp >=================================================================== >RCS file: /cvsroot/mozilla/docshell/base/nsWebShell.cpp,v >retrieving revision 1.681 >diff -u -8 -p -r1.681 nsWebShell.cpp >--- base/nsWebShell.cpp 28 Nov 2006 20:47:10 -0000 1.681 >+++ base/nsWebShell.cpp 28 Nov 2006 23:24:42 -0000 >@@ -117,16 +117,17 @@ > #include "nsCExternalHandlerService.h" > > #include "nsIIDNService.h" > > #include "nsIPrefBranch.h" > #include "nsIPrefService.h" > #include "nsITimer.h" > #include "nsIScriptSecurityManager.h" >+#include "nsContentPolicyUtils.h" > > #ifdef NS_DEBUG > /** > * Note: the log module is created during initialization which > * means that you cannot perform logging before then. > */ > static PRLogModuleInfo* gLogModule = PR_NewLogModule("webshell"); > #endif >@@ -187,16 +188,60 @@ PingsEnabled(PRInt32 *maxPerLink, PRBool > prefs->GetIntPref(PREF_PINGS_MAX_PER_LINK, maxPerLink); > prefs->GetBoolPref(PREF_PINGS_REQUIRE_SAME_HOST, requireSameHost); > } > } > > return allow; > } > >+static PRBool - In CheckPingURI(): ... + if (NS_FAILED(rv) || NS_CP_REJECTED(shouldLoad)) { + return PR_FALSE; + } +} Missing return at the end of the function. Maybe just return NS_SUCCEEDED(rv) && !NS_CP_REJECTED(shouldLoad) and avoid the explicit if check? r+sr=jst with that fixed.
Attachment #246862 -
Flags: superreview?(jst)
Attachment #246862 -
Flags: superreview+
Attachment #246862 -
Flags: review?(jst)
Attachment #246862 -
Flags: review+
Assignee | ||
Comment 15•18 years ago
|
||
Checked in with that fixed
Status: REOPENED → RESOLVED
Closed: 18 years ago → 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•