337755 - IsCallerSecure() implementation incorrect

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Comment 1

[Boris Zbarsky [:bzbarsky]]

Er... So what I _meant_ to say was:

The IsCallerSecure() impl in dom/src/storage/nsDOMStorage.cpp will break for jar:https:// sites.  It will also break for other protocols that are secure (imap: can be, for example), but that worries me less.

On branch, you probably have to QI to nsIJARURI, etc.  On trunk, you can use NS_GetInnermostURI or better yet, imo, add a protocol handler flag for determining whether a given protocol is secure and use NS_URIChainHasFlags.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

so does this base secureness determination only on the scheme? sounds like a bad idea to me... what if a protocol can be both secure and insecure, depending on what you negotiate?

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

IMO, this should use the channel's securityInfo, or something equivalent (docshell's securityUI?)

Comment 4

[Boris Zbarsky [:bzbarsky]]

That might be a good idea... Eg. if we have an https:// URI that includes insecure content (so broken lock icon), I'm not sure we want to treat it as "secure" here.

Comment 5

[Johnny Stenback (:jst)]

Attachment: Make IsCallerSecure() deal with jar: URIs etc.

Yes, long-term we'd want something like that for sure, but that's a rather large change here and to the code related to our handling of secure and mixed https pages. This is a trivial patch that gets us a far bit further than the current code at least.

Comment 6

[Johnny Stenback (:jst)]

Attachment: Make IsCallerSecure() deal with jar: URIs etc.

Complete diff this time, the previous one was a partial change.

Comment 7

[Johnny Stenback (:jst)]

Attachment: Make IsCallerSecure() deal with jar: URIs etc.

Ok, this is the right diff, really.

Comment 8

[Johnny Stenback (:jst)]

Fix landed on the trunk.

Comment 9

[Johnny Stenback (:jst)]

Attachment: Branch version of the same, in case we want it.

This is the same as the above change, only it uses nsIJARURI direcly since there is no NS_GetInnermostURI() on the branch.

Comment 10

[Johnny Stenback (:jst)]

This is IMO low-risk enough, even if not extremely high benefit, to take on the branch for 2.0.

Comment 11

[Johnny Stenback (:jst)]

Fixed on the 1.8 branch.