Closed Bug 462800 Opened 14 years ago Closed 14 years ago

[FIX]Inconsistent domains in DOM storage

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
All
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9.1b3

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

(Keywords: fixed1.9.1)

Attachments

(1 file)

Let's just fix this
4.54 KB, patch
dcamp
: review+
sicking
: superreview+
beltzner
: approval1.9.1+
Details | Diff | Splinter Review 
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.
Flags: blocking1.9.1?
Jonas claims this shouldn't lead to security problems. Minusing based on that, please renominate if this is indeed a serious risk.
Flags: blocking1.9.1? → blocking1.9.1-
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.
Flags: wanted1.9.1?
Flags: wanted1.9.1? → wanted1.9.1+
Priority: -- → P1
This also fixes bug 462801.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #346822 - Flags: superreview?(jonas)
Attachment #346822 - Flags: review?(dcamp)
Summary: Inconsistent domains in DOM storage → [FIX]Inconsistent domains in DOM storage
This does also make one behavior change: passing the innermost URI to the permission manager.  I believe this is what we want anyway.
Attachment #346822 - Flags: review?(dcamp) → review+
Attachment #346822 - Flags: superreview?(jonas) → superreview+
Attachment #346822 - Flags: approval1.9.1?
Comment on attachment 346822 [details] [diff] [review]
Let's just fix this

I think we should take this for 1.9.1...
Comment on attachment 346822 [details] [diff] [review]
Let's just fix this

a191=beltzner
Attachment #346822 - Flags: approval1.9.1? → approval1.9.1+
Blocks: 462801
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.  :(
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
Keywords: fixed1.9.1
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.