Closed Bug 1121765 Opened 5 years ago Closed 5 years ago

NetworkHelper.parseSecurityInfo threw an exception: Could not get HSTS/HPKP status as request.URI not available.

Categories

(DevTools :: Netmonitor, defect)

x86
macOS
defect
Not set

Tracking

(e10s+)

RESOLVED FIXED
Firefox 38
Tracking Status
e10s + ---

People

(Reporter: cpeterson, Assigned: sjakthol)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [e10s-m6])

Attachments

(1 file, 1 obsolete file)

I see the following error messages in the browser console when logging into Yammer using Okta SSO when e10s is enabled:

https://www.yammer.com/mozilla.com

NetworkHelper.parseSecurityInfo threw an exception: Could not get HSTS/HPKP status as request.URI not available. DevToolsUtils.js:58:0
NetworkHelper.parseSecurityInfo threw an exception: Could not get HSTS/HPKP status as request.URI not available. DevToolsUtils.js:58:0
This doesn't seem to be e10s-related, after all. I see the same errors on Yammer without e10s.
Summary: [e10s] NetworkHelper.parseSecurityInfo threw an exception: Could not get HSTS/HPKP status as request.URI not available. → NetworkHelper.parseSecurityInfo threw an exception: Could not get HSTS/HPKP status as request.URI not available.
That message was meant to be shown as warning if nsIRequest does not have an URI attached to it. Apparently this is more common than I thought.

Sadly I don't have credentials required to reproduce what you're seeing. I suspect this is caused by multiple consecutive redirects that caused some other issues while I was working on bug 932179. It could be that by the time we get securityInfo for a request (nsIRequestObserver.onStartResponse) the URI of the related nsIRequest is already destroyed.

If the URI is available some point in time (i.e. there's no requests without domain name in netmonitor when the exception occurs), it should be possible to save the hostname there and use it when we finally check the security status (option #1).

Another option is to just remove the warning altogether if the case it's triggered turns out to be a lot more common than I thought (option #2).

I'll look into option #1.
Status: NEW → ASSIGNED
Component: Developer Tools → Developer Tools: Netmonitor
Assignee: nobody → sjakthol
Here's a patch that saves the hostname early on in NetworkMonitor.createActivityObject and uses that to query HSTS/HPKP status later when parsing security information. If the URI is not available in _onRequestHeader, we should be doomed long before we get to a point where security info would be parsed.

I also changed NetworkHelper.parseSecurityInfo to take the httpActivity object instead of nsIRequest as the second parameter.

Apparently with e10s requests don't have a window attached to them so private browsing was not properly detected. This patch makes the monitor check for private browsing in two different ways:
1) nsIPrivateBrowsingChannel.isChannelPrivate
2) PrivateBrowsingUtils.isWindowPrivate

If either of those checks indicate a private request, it's marked to be private.

I left the isPrivateWindow check intact as it was added for Firefox OS and I don't know if isChannelPrivate works there. If it does the isPrivateWindow check should probably be removed.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3ba90256e9f
Attachment #8550473 - Flags: review?(past)
Comment on attachment 8550473 [details] [diff] [review]
netmonitor-missing-request-URI.patch

Review of attachment 8550473 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, thanks for the detailed explanation Sami!

(In reply to Sami Jaktholm from comment #3)
> I left the isPrivateWindow check intact as it was added for Firefox OS and I
> don't know if isChannelPrivate works there. If it does the isPrivateWindow
> check should probably be removed.

Alex, do you know if channel.isChannelPrivate works on b2g?

::: toolkit/devtools/webconsole/network-monitor.js
@@ +667,5 @@
> +    let channelPrivate = aChannel.isChannelPrivate;
> +
> +    // win is not available in e10s. Use channel instead.
> +    let winPrivate = win ? PrivateBrowsingUtils.isWindowPrivate(win) : false;
> +    

Nit: trailing whitespace.
Attachment #8550473 - Flags: review?(past)
Attachment #8550473 - Flags: review+
Attachment #8550473 - Flags: feedback?(poirot.alex)
Yes, isChannelPrivate is e10s-aware.
(In reply to Josh Matthews [:jdm] from comment #5)
> Yes, isChannelPrivate is e10s-aware.

Does that imply it is also b2g-aware? Note that PrivateBrowsingUtils.isWindowPrivate was kept for b2g compatibility, not e10s.
I'm not sure what being b2g-aware means in this context.
(In reply to Josh Matthews [:jdm] from comment #7)
> I'm not sure what being b2g-aware means in this context.

On b2g if I QI a nsIHttpChannel into nsIPrivateBrowsingChannel and check its isChannelPrivate will it tell me
* true if that channel is related to (or opened from) a private window
* false if that channel was not initiated from a private window?

I'm not really familiar with b2g so I hope that's clear enough.
Yes.
Comment on attachment 8550473 [details] [diff] [review]
netmonitor-missing-request-URI.patch

Thanks Josh.
Attachment #8550473 - Flags: feedback?(poirot.alex)
All right, thanks.

Here's a version that removes PrivateBrowsingUtils.isWindowPrivate from netmonitor and relies on nsIPrivateBrowsingChannel.isChannelPrivate on all platforms.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bca24e43973b
Attachment #8550473 - Attachment is obsolete: true
Attachment #8551738 - Flags: review?(past)
Attachment #8551738 - Flags: review?(past) → review+
Keywords: checkin-needed
Duplicate of this bug: 1079805
https://hg.mozilla.org/integration/fx-team/rev/11942c706677
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [e10s-m6] → [e10s-m6][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/11942c706677
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [e10s-m6][fixed-in-fx-team] → [e10s-m6]
Target Milestone: --- → Firefox 38
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.