Closed
Bug 1110475
Opened 10 years ago
Closed 10 years ago
Fetch Request should not require origin to be set
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
INVALID
People
(Reporter: bkelly, Unassigned)
Details
This bug it track removing the origin field from Fetch Request. This is something Nikhil and I discussed over email, but I never got around to implementing on maple. Now that we're moving to mozilla-central I thought I better write it up as a bug.
From the email:
Basically, I'm having trouble setting the InternalRequest origin value for all principals. Talking to Jonas it appears that we probably don't need Request to have an origin at all.
The two places its currently used are in FetchDriver.cpp for:
1) To do the same origin check like this:
nsAutoCString originURL;
mRequest->GetOrigin(originURL);
nsCOMPtr<nsIURI> originURI;
rv = NS_NewURI(getter_AddRefs(originURI), originURL, nullptr, nullptr);
if (NS_WARN_IF(NS_FAILED(rv))) {
return FailWithNetworkError();
}
nsIScriptSecurityManager* ssm = nsContentUtils::GetSecurityManager();
rv = ssm->CheckSameOriginURI(requestURI, originURI, false);
2) Set the origin header:
if (mRequest->ForceOriginHeader()) {
nsCString origin;
mRequest->GetOrigin(origin);
httpChan->SetRequestHeader(NS_LITERAL_CSTRING("origin"),
origin,
false /* merge */);
}
Jonas suggested that in both cases we probably should be use the page's principal instead. So instead of calling mRequest->GetOrigin() you would query mPrincipal instead.
In the case of the same origin check, there is also a better way to do the same origin check:
mPrincipal->CheckMayLoad(uri);
Also note that the origin may not be valid for all principals. For example, a system principal returns "[System Principal]" instead of a URI.
Does dropping the InternalRequest origin member sound reasonable?
I have integrated these changes into the fetch patches. Got rid of the origin and expect that whoever has an InternalRequest and wants to fetch a URL also has a valid principal.
Reporter | ||
Comment 2•10 years ago
|
||
Are these patches uploaded anywhere in particular? I need to adapt the Cache patches to match.
Flags: needinfo?(nsm.nikhil)
Not yet, but it just removes the origin field. I already had the requirement that FetchDriver be passed a principal. That bit is in patch 5 of bug dom-fetch-api
Flags: needinfo?(nsm.nikhil)
Reporter | ||
Comment 4•10 years ago
|
||
Ok, just be aware that P1 patch in bug 940273 currently adds a SetOrigin() method. I'll have to update this when you refresh dom-fetch-api patches.
Reporter | ||
Comment 5•10 years ago
|
||
I ended up doing this in bug 940273 in a rebase. So this bug is invalid.
No longer blocks: serviceworker-cache
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → INVALID
Assignee | ||
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
•