Call CheckLoadURI before issuing pings

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
Document Navigation
P2
major
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: Darin Fisher, Assigned: sicking)

Tracking

Trunk
mozilla1.9alpha1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [blocking1.9a1+] if a ping is on by default)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

12 years ago
Call CheckLoadURI before issuing pings

We should restrict pings as we restrict redirects.
(Reporter)

Updated

12 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.9alpha
Flags: blocking1.9a1+
I think it should also check with content policy (::ShouldProcess?), so content policy can selectively veto the ping.
(Reporter)

Comment 2

12 years ago
Yeah, dveditz made the same comment to me over IM.  I plan to include that in the forthcoming patch.
(Reporter)

Comment 3

12 years ago
*** Bug 323959 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 4

12 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
that won't fix the content policy part of this bug.
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)

Comment 7

12 years ago
ok
(Reporter)

Updated

12 years ago
Priority: -- → P2
Flags: blocking1.9+
Flags: blocking1.9a1+
Whiteboard: [blocking1.9a1+] if a ping is on by default
Assignee: darin.moz → bugmail
Status: ASSIGNED → NEW
Created attachment 246722 [details] [diff] [review]
Patch to fix

This should do it
Attachment #246722 - Flags: superreview?(jst)
Attachment #246722 - Flags: review?(jst)
Created attachment 246724 [details] [diff] [review]
patch v2

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 on attachment 246722 [details] [diff] [review]
Patch to fix

r+sr=jst
Attachment #246722 - Flags: superreview+
Attachment #246722 - Flags: review+

Updated

11 years ago
Attachment #246724 - Flags: superreview?(jst)
Attachment #246724 - Flags: superreview+
Attachment #246724 - Flags: review?(jst)
Attachment #246724 - Flags: review+

Updated

11 years ago
Attachment #246722 - Flags: superreview+
Attachment #246722 - Flags: review+
Checked in
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Ugh, missed the comments at the top of this bug, there's some more work to be done here.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 246862 [details] [diff] [review]
This one's better

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 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+
Checked in with that fixed
Status: REOPENED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED

Updated

10 years ago
Blocks: 323959
You need to log in before you can comment on or make changes to this bug.