Audit the callers of the two-argument OriginAttributes constructor
RESOLVED
FIXED
in Firefox 46
Status
()
People
(Reporter: bholley, Assigned: allstars.chh)
Tracking
Firefox Tracking Flags
(firefox46 fixed)
Details
Attachments
(1 attachment, 1 obsolete attachment)
5.91 KB,
patch
|
allstars.chh
:
review+
|
Details | Diff | Splinter Review |
Looking through the code, I see a lot of callers that follow the letter of OriginAttributes but not the spirit: https://dxr.mozilla.org/mozilla-central/rev/6256ec9113c115141aab089c45ee69438884b680/dom/quota/QuotaManager.cpp#5291 Stuff like that isn't going to work when we add signedPkg etc. We should audit and fix these consumers, and possibly remove that constructor.
(Assignee) | ||
Updated•3 years ago
|
Assignee: nobody → allstars.chh
(Assignee) | ||
Comment 1•3 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b08d94df2937
(Assignee) | ||
Comment 2•3 years ago
|
||
Created attachment 8699420 [details] [diff] [review] Patch.
(Assignee) | ||
Comment 3•3 years ago
|
||
Comment on attachment 8699420 [details] [diff] [review] Patch. Review of attachment 8699420 [details] [diff] [review]: ----------------------------------------------------------------- Hi, Janv Could you help to review QuotaManager part for me? I only updated OriginProps and some code in OriginParser. Thanks
Attachment #8699420 -
Flags: review?(jvarga)
Comment 4•3 years ago
|
||
Comment on attachment 8699420 [details] [diff] [review] Patch. Review of attachment 8699420 [details] [diff] [review]: ----------------------------------------------------------------- r=me if you address my comments ::: dom/quota/ActorsParent.cpp @@ +5988,5 @@ > nsCString spec; > + PrincipalOriginAttributes attrs; > + bool result = OriginParser::ParseOrigin(NS_ConvertUTF16toUTF8(leafName), > + spec, &attrs); > + NS_ENSURE_TRUE(result, NS_ERROR_FAILURE); please don't use NS_ENSURE_TRUE(), see my comment below @@ +6170,5 @@ > } > > + nsCOMPtr<nsIPrincipal> principal = > + BasePrincipal::CreateCodebasePrincipal(uri, originProps.mAttrs); > + NS_ENSURE_TRUE(principal, NS_ERROR_FAILURE); please no more NS_ENSURE_TRUE(), use this instead: if (NS_WARN_IF(!principal)) { return NS_ERROR_FAILURE; }
Attachment #8699420 -
Flags: review?(jvarga) → review+
(Assignee) | ||
Comment 5•3 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba6ecf578741
(Assignee) | ||
Comment 6•3 years ago
|
||
Created attachment 8699808 [details] [diff] [review] Patch. v2 addressed janv's comment.
Attachment #8699420 -
Attachment is obsolete: true
Attachment #8699808 -
Flags: review+
(Assignee) | ||
Comment 7•3 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/63254aa6dfabeced552c9401cf9fb3cfa9b7add4 Bug 1209349 - Audit the callers of the two-argument OriginAttributes. r=janv
Comment 8•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/63254aa6dfab
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in
before you can comment on or make changes to this bug.
Description
•