462800 - [FIX]Inconsistent domains in DOM storage

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

nsDOMStorage::SetDBValue uses the URI of the principal for GetAsciiHost.

nsDOMStorageList::NamedItem uses the domain of the principal, falling back on the URI.

The latter was changed from being consistent with the former in bug 424093.

I suspect we want the NamedItem behavior throughout.  Ideally we would be using a helper function that gets an ASCII-fied hostname from a principal instead of copy/pasting the code.

Nominating, since inconsistent security checks are usually a bad idea.  Not sure whether we need this on branch, though; someone who understands the security implications of the differences would need to look into that.

Comment 1

[Johnny Stenback (:jst)]

Jonas claims this shouldn't lead to security problems. Minusing based on that, please renominate if this is indeed a serious risk.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I have no idea; I'm not really going to spend time reading through the code trying to sort this out.  I just think we should fix this, or at least sort out whether it needs fixing.  Maybe the inconsistency is actually correct.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Let's just fix this

This also fixes bug 462801.

Comment 4

[Boris Zbarsky [:bzbarsky]]

This does also make one behavior change: passing the innermost URI to the permission manager.  I believe this is what we want anyway.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 346822 [details] [diff] [review]
Let's just fix this

I think we should take this for 1.9.1...

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 346822 [details] [diff] [review]
Let's just fix this

a191=beltzner

Comment 7

[Boris Zbarsky [:bzbarsky]]

Pushed fix as http://hg.mozilla.org/mozilla-central/rev/f36ed9fffc2c and then pushed followup http://hg.mozilla.org/mozilla-central/rev/beb23427d11a to fix test bustage because I had the sense of the null-check reversed.

Not sure how to test this stuff offhand.  :(