Closed
Bug 462800
Opened 17 years ago
Closed 17 years ago
[FIX]Inconsistent domains in DOM storage
Categories
(Core :: DOM: Core & HTML, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla1.9.1b3
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
(Keywords: fixed1.9.1)
Attachments
(1 file)
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?
Comment 1•17 years ago
|
||
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-
![]() |
Assignee | |
Comment 2•17 years ago
|
||
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?
Updated•17 years ago
|
Flags: wanted1.9.1? → wanted1.9.1+
Priority: -- → P1
![]() |
Assignee | |
Comment 3•17 years ago
|
||
This also fixes bug 462801.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #346822 -
Flags: superreview?(jonas)
Attachment #346822 -
Flags: review?(dcamp)
![]() |
Assignee | |
Updated•17 years ago
|
Summary: Inconsistent domains in DOM storage → [FIX]Inconsistent domains in DOM storage
![]() |
Assignee | |
Comment 4•17 years ago
|
||
This does also make one behavior change: passing the innermost URI to the permission manager. I believe this is what we want anyway.
Updated•17 years ago
|
Attachment #346822 -
Flags: review?(dcamp) → review+
Attachment #346822 -
Flags: superreview?(jonas) → superreview+
![]() |
Assignee | |
Updated•17 years ago
|
Attachment #346822 -
Flags: approval1.9.1?
![]() |
Assignee | |
Comment 5•17 years ago
|
||
Comment on attachment 346822 [details] [diff] [review]
Let's just fix this
I think we should take this for 1.9.1...
Comment 6•17 years ago
|
||
Comment on attachment 346822 [details] [diff] [review]
Let's just fix this
a191=beltzner
Attachment #346822 -
Flags: approval1.9.1? → approval1.9.1+
![]() |
Assignee | |
Comment 7•17 years ago
|
||
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: 17 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
![]() |
Assignee | |
Updated•17 years ago
|
Keywords: fixed1.9.1
Updated•12 years ago
|
Component: DOM: Mozilla Extensions → DOM
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•