Skip to content

Commit

Permalink
Bug 1319111 - Expose 'result principal URI' on LoadInfo as a source f…
Browse files Browse the repository at this point in the history
…or NS_GetFinalChannelURI (removes some use of LOAD_REPLACE flag). r=bz
  • Loading branch information
mayhemer committed May 23, 2017
1 parent c288415 commit b59d80d
Show file tree
Hide file tree
Showing 25 changed files with 255 additions and 129 deletions.
18 changes: 8 additions & 10 deletions browser/components/about/AboutRedirector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ AboutRedirector::NewChannel(nsIURI* aURI,
nsIChannel** result)
{
NS_ENSURE_ARG_POINTER(aURI);
NS_ENSURE_ARG_POINTER(aLoadInfo);

NS_ASSERTION(result, "must not be null");

nsAutoCString path = GetAboutModuleName(aURI);
Expand Down Expand Up @@ -172,26 +174,22 @@ AboutRedirector::NewChannel(nsIURI* aURI,
NS_ENSURE_SUCCESS(rv, rv);

// If tempURI links to an external URI (i.e. something other than
// chrome:// or resource://) then set the LOAD_REPLACE flag on the
// channel which forces the channel owner to reflect the displayed
// chrome:// or resource://) then set the result principal URI on the
// load info which forces the channel prncipal to reflect the displayed
// URL rather then being the systemPrincipal.
bool isUIResource = false;
rv = NS_URIChainHasFlags(tempURI, nsIProtocolHandler::URI_IS_UI_RESOURCE,
&isUIResource);
NS_ENSURE_SUCCESS(rv, rv);

nsLoadFlags loadFlags = isUIResource
? static_cast<nsLoadFlags>(nsIChannel::LOAD_NORMAL)
: static_cast<nsLoadFlags>(nsIChannel::LOAD_REPLACE);

rv = NS_NewChannelInternal(getter_AddRefs(tempChannel),
tempURI,
aLoadInfo,
nullptr, // aLoadGroup
nullptr, // aCallbacks
loadFlags);
aLoadInfo);
NS_ENSURE_SUCCESS(rv, rv);

if (!isUIResource) {
aLoadInfo->SetResultPrincipalURI(tempURI);
}
tempChannel->SetOriginalURI(aURI);

NS_ADDREF(*result = tempChannel);
Expand Down
13 changes: 10 additions & 3 deletions chrome/nsChromeProtocolHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ nsChromeProtocolHandler::NewChannel2(nsIURI* aURI,
nsresult rv;

NS_ENSURE_ARG_POINTER(aURI);
NS_ENSURE_ARG_POINTER(aLoadInfo);

NS_PRECONDITION(aResult, "Null out param");

#ifdef DEBUG
Expand Down Expand Up @@ -145,6 +147,12 @@ nsChromeProtocolHandler::NewChannel2(nsIURI* aURI,
return rv;
}

// We don't want to allow the inner protocol handler modify the result principal URI
// since we want either |aURI| or anything pre-set by upper layers to prevail.
nsCOMPtr<nsIURI> savedResultPrincipalURI;
rv = aLoadInfo->GetResultPrincipalURI(getter_AddRefs(savedResultPrincipalURI));
NS_ENSURE_SUCCESS(rv, rv);

rv = NS_NewChannelInternal(getter_AddRefs(result),
resolvedURI,
aLoadInfo);
Expand All @@ -168,9 +176,8 @@ nsChromeProtocolHandler::NewChannel2(nsIURI* aURI,

// Make sure that the channel remembers where it was
// originally loaded from.
nsLoadFlags loadFlags = 0;
result->GetLoadFlags(&loadFlags);
result->SetLoadFlags(loadFlags & ~nsIChannel::LOAD_REPLACE);
rv = aLoadInfo->SetResultPrincipalURI(savedResultPrincipalURI);
NS_ENSURE_SUCCESS(rv, rv);
rv = result->SetOriginalURI(aURI);
if (NS_FAILED(rv)) return rv;

Expand Down
1 change: 1 addition & 0 deletions devtools/client/framework/about-devtools-toolbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ AboutURL.prototype = {
newChannel: function (aURI, aLoadInfo) {
let chan = Services.io.newChannelFromURIWithLoadInfo(this.uri, aLoadInfo);
chan.owner = Services.scriptSecurityManager.getSystemPrincipal();
chan.originalURI = aURI;
return chan;
},

Expand Down
24 changes: 11 additions & 13 deletions docshell/base/nsAboutRedirector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ nsAboutRedirector::NewChannel(nsIURI* aURI,
nsIChannel** aResult)
{
NS_ENSURE_ARG_POINTER(aURI);
NS_ENSURE_ARG_POINTER(aLoadInfo);
NS_ASSERTION(aResult, "must not be null");

nsAutoCString path;
Expand All @@ -174,9 +175,14 @@ nsAboutRedirector::NewChannel(nsIURI* aURI,
rv = NS_NewURI(getter_AddRefs(tempURI), kRedirMap[i].url);
NS_ENSURE_SUCCESS(rv, rv);

rv = NS_NewChannelInternal(getter_AddRefs(tempChannel),
tempURI,
aLoadInfo);
NS_ENSURE_SUCCESS(rv, rv);

// If tempURI links to an external URI (i.e. something other than
// chrome:// or resource://) then set the LOAD_REPLACE flag on the
// channel which forces the channel owner to reflect the displayed
// chrome:// or resource://) then set result principal URI on the
// load info which forces the channel principal to reflect the displayed
// URL rather then being the systemPrincipal.
bool isUIResource = false;
rv = NS_URIChainHasFlags(tempURI, nsIProtocolHandler::URI_IS_UI_RESOURCE,
Expand All @@ -185,17 +191,9 @@ nsAboutRedirector::NewChannel(nsIURI* aURI,

bool isAboutBlank = NS_IsAboutBlank(tempURI);

nsLoadFlags loadFlags = isUIResource || isAboutBlank
? static_cast<nsLoadFlags>(nsIChannel::LOAD_NORMAL)
: static_cast<nsLoadFlags>(nsIChannel::LOAD_REPLACE);

rv = NS_NewChannelInternal(getter_AddRefs(tempChannel),
tempURI,
aLoadInfo,
nullptr, // aLoadGroup
nullptr, // aCallbacks
loadFlags);
NS_ENSURE_SUCCESS(rv, rv);
if (!isUIResource && !isAboutBlank) {
aLoadInfo->SetResultPrincipalURI(tempURI);
}

tempChannel->SetOriginalURI(aURI);

Expand Down
6 changes: 6 additions & 0 deletions docshell/base/nsDocShell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11182,6 +11182,12 @@ nsDocShell::DoURILoad(nsIURI* aURI,

if (aOriginalURI) {
channel->SetOriginalURI(aOriginalURI);
// The LOAD_REPLACE flag and its handling here will be removed as part
// of bug 1319110. For now preserve its restoration here to not break
// any code expecting it being set specially on redirected channels.
// If the flag has originally been set to change result of
// NS_GetFinalChannelURI it won't have any effect and also won't cause
// any harm.
if (aLoadReplace) {
uint32_t loadFlags;
channel->GetLoadFlags(&loadFlags);
Expand Down
16 changes: 16 additions & 0 deletions ipc/glue/BackgroundUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "nsContentUtils.h"
#include "nsString.h"
#include "nsTArray.h"
#include "URIUtils.h"

namespace mozilla {
namespace net {
Expand Down Expand Up @@ -304,6 +305,13 @@ LoadInfoToLoadInfoArgs(nsILoadInfo *aLoadInfo,
sandboxedLoadingPrincipalInfo = sandboxedLoadingPrincipalInfoTemp;
}

OptionalURIParams optionalResultPrincipalURI = mozilla::void_t();
nsCOMPtr<nsIURI> resultPrincipalURI;
Unused << aLoadInfo->GetResultPrincipalURI(getter_AddRefs(resultPrincipalURI));
if (resultPrincipalURI) {
SerializeURI(resultPrincipalURI, optionalResultPrincipalURI);
}

nsTArray<PrincipalInfo> redirectChainIncludingInternalRedirects;
for (const nsCOMPtr<nsIPrincipal>& principal : aLoadInfo->RedirectChainIncludingInternalRedirects()) {
rv = PrincipalToPrincipalInfo(principal, redirectChainIncludingInternalRedirects.AppendElement());
Expand All @@ -322,6 +330,7 @@ LoadInfoToLoadInfoArgs(nsILoadInfo *aLoadInfo,
triggeringPrincipalInfo,
principalToInheritInfo,
sandboxedLoadingPrincipalInfo,
optionalResultPrincipalURI,
aLoadInfo->GetSecurityFlags(),
aLoadInfo->InternalContentPolicyType(),
static_cast<uint32_t>(aLoadInfo->GetTainting()),
Expand Down Expand Up @@ -385,6 +394,12 @@ LoadInfoArgsToLoadInfo(const OptionalLoadInfoArgs& aOptionalLoadInfoArgs,
NS_ENSURE_SUCCESS(rv, rv);
}

nsCOMPtr<nsIURI> resultPrincipalURI;
if (loadInfoArgs.resultPrincipalURI().type() != OptionalURIParams::Tvoid_t) {
resultPrincipalURI = DeserializeURI(loadInfoArgs.resultPrincipalURI());
NS_ENSURE_TRUE(resultPrincipalURI, NS_ERROR_UNEXPECTED);
}

nsTArray<nsCOMPtr<nsIPrincipal>> redirectChainIncludingInternalRedirects;
for (const PrincipalInfo& principalInfo : loadInfoArgs.redirectChainIncludingInternalRedirects()) {
nsCOMPtr<nsIPrincipal> redirectedPrincipal =
Expand All @@ -406,6 +421,7 @@ LoadInfoArgsToLoadInfo(const OptionalLoadInfoArgs& aOptionalLoadInfoArgs,
triggeringPrincipal,
principalToInherit,
sandboxedLoadingPrincipal,
resultPrincipalURI,
loadInfoArgs.securityFlags(),
loadInfoArgs.contentPolicyType(),
static_cast<LoadTainting>(loadInfoArgs.tainting()),
Expand Down
7 changes: 5 additions & 2 deletions modules/libjar/nsJARChannel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -920,8 +920,8 @@ nsJARChannel::OnDownloadComplete(MemoryDownloader* aDownloader,
uint32_t loadFlags;
channel->GetLoadFlags(&loadFlags);
if (loadFlags & LOAD_REPLACE) {
mLoadFlags |= LOAD_REPLACE;

// Update our URI to reflect any redirects that happen during
// the HTTP request.
if (!mOriginalURI) {
SetOriginalURI(mJarURI);
}
Expand All @@ -943,6 +943,9 @@ nsJARChannel::OnDownloadComplete(MemoryDownloader* aDownloader,
}

if (NS_SUCCEEDED(status) && channel) {
// In case the load info object has changed during a redirect,
// grab it from the target channel.
channel->GetLoadInfo(getter_AddRefs(mLoadInfo));
// Grab the security info from our base channel
channel->GetSecurityInfo(getter_AddRefs(mSecurityInfo));

Expand Down
17 changes: 17 additions & 0 deletions netwerk/base/LoadInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ LoadInfo::LoadInfo(const LoadInfo& rhs)
, mTriggeringPrincipal(rhs.mTriggeringPrincipal)
, mPrincipalToInherit(rhs.mPrincipalToInherit)
, mSandboxedLoadingPrincipal(rhs.mSandboxedLoadingPrincipal)
, mResultPrincipalURI(rhs.mResultPrincipalURI)
, mLoadingContext(rhs.mLoadingContext)
, mSecurityFlags(rhs.mSecurityFlags)
, mInternalContentPolicyType(rhs.mInternalContentPolicyType)
Expand Down Expand Up @@ -300,6 +301,7 @@ LoadInfo::LoadInfo(nsIPrincipal* aLoadingPrincipal,
nsIPrincipal* aTriggeringPrincipal,
nsIPrincipal* aPrincipalToInherit,
nsIPrincipal* aSandboxedLoadingPrincipal,
nsIURI* aResultPrincipalURI,
nsSecurityFlags aSecurityFlags,
nsContentPolicyType aContentPolicyType,
LoadTainting aTainting,
Expand All @@ -325,6 +327,7 @@ LoadInfo::LoadInfo(nsIPrincipal* aLoadingPrincipal,
: mLoadingPrincipal(aLoadingPrincipal)
, mTriggeringPrincipal(aTriggeringPrincipal)
, mPrincipalToInherit(aPrincipalToInherit)
, mResultPrincipalURI(aResultPrincipalURI)
, mSecurityFlags(aSecurityFlags)
, mInternalContentPolicyType(aContentPolicyType)
, mTainting(aTainting)
Expand Down Expand Up @@ -936,5 +939,19 @@ LoadInfo::GetIsTopLevelLoad(bool *aResult)
return NS_OK;
}

NS_IMETHODIMP
LoadInfo::GetResultPrincipalURI(nsIURI **aURI)
{
NS_IF_ADDREF(*aURI = mResultPrincipalURI);
return NS_OK;
}

NS_IMETHODIMP
LoadInfo::SetResultPrincipalURI(nsIURI *aURI)
{
mResultPrincipalURI = aURI;
return NS_OK;
}

} // namespace net
} // namespace mozilla
2 changes: 2 additions & 0 deletions netwerk/base/LoadInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class LoadInfo final : public nsILoadInfo
nsIPrincipal* aTriggeringPrincipal,
nsIPrincipal* aPrincipalToInherit,
nsIPrincipal* aSandboxedLoadingPrincipal,
nsIURI* aResultPrincipalURI,
nsSecurityFlags aSecurityFlags,
nsContentPolicyType aContentPolicyType,
LoadTainting aTainting,
Expand Down Expand Up @@ -128,6 +129,7 @@ class LoadInfo final : public nsILoadInfo
nsCOMPtr<nsIPrincipal> mTriggeringPrincipal;
nsCOMPtr<nsIPrincipal> mPrincipalToInherit;
nsCOMPtr<nsIPrincipal> mSandboxedLoadingPrincipal;
nsCOMPtr<nsIURI> mResultPrincipalURI;
nsWeakPtr mLoadingContext;
nsSecurityFlags mSecurityFlags;
nsContentPolicyType mInternalContentPolicyType;
Expand Down
9 changes: 9 additions & 0 deletions netwerk/base/nsILoadInfo.idl
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
interface nsIDOMDocument;
interface nsINode;
interface nsIPrincipal;
interface nsIURI;

%{C++
#include "nsTArray.h"
Expand Down Expand Up @@ -740,6 +741,14 @@ interface nsILoadInfo : nsISupports
*/
[infallible] readonly attribute boolean isTopLevelLoad;

/**
* If this is non-null, this property represents two things: (1) the
* URI to be used for the principal if the channel with this loadinfo
* gets a principal based on URI and (2) the URI to use for a document
* created from the channel with this loadinfo.
*/
attribute nsIURI resultPrincipalURI;

/**
* Returns the null principal of the resulting resource if the SEC_SANDBOXED
* flag is set. Otherwise returns null. This is used by
Expand Down
41 changes: 28 additions & 13 deletions netwerk/base/nsNetUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,17 @@ NS_NewChannelInternal(nsIChannel **outChannel,
NS_ENSURE_SUCCESS(rv, rv);
}

#ifdef DEBUG
nsLoadFlags channelLoadFlags = 0;
channel->GetLoadFlags(&channelLoadFlags);
// Will be removed when we remove LOAD_REPLACE altogether
// This check is trying to catch protocol handlers that still
// try to set the LOAD_REPLACE flag.
MOZ_DIAGNOSTIC_ASSERT(!(channelLoadFlags & nsIChannel::LOAD_REPLACE));
#endif

if (aLoadFlags != nsIRequest::LOAD_NORMAL) {
// Retain the LOAD_REPLACE load flag if set.
nsLoadFlags normalLoadFlags = 0;
channel->GetLoadFlags(&normalLoadFlags);
rv = channel->SetLoadFlags(aLoadFlags | (normalLoadFlags & nsIChannel::LOAD_REPLACE));
rv = channel->SetLoadFlags(aLoadFlags);
NS_ENSURE_SUCCESS(rv, rv);
}

Expand Down Expand Up @@ -267,11 +273,17 @@ NS_NewChannelInternal(nsIChannel **outChannel,
NS_ENSURE_SUCCESS(rv, rv);
}

#ifdef DEBUG
nsLoadFlags channelLoadFlags = 0;
channel->GetLoadFlags(&channelLoadFlags);
// Will be removed when we remove LOAD_REPLACE altogether
// This check is trying to catch protocol handlers that still
// try to set the LOAD_REPLACE flag.
MOZ_DIAGNOSTIC_ASSERT(!(channelLoadFlags & nsIChannel::LOAD_REPLACE));
#endif

if (aLoadFlags != nsIRequest::LOAD_NORMAL) {
// Retain the LOAD_REPLACE load flag if set.
nsLoadFlags normalLoadFlags = 0;
channel->GetLoadFlags(&normalLoadFlags);
rv = channel->SetLoadFlags(aLoadFlags | (normalLoadFlags & nsIChannel::LOAD_REPLACE));
rv = channel->SetLoadFlags(aLoadFlags);
NS_ENSURE_SUCCESS(rv, rv);
}

Expand Down Expand Up @@ -1885,12 +1897,15 @@ nsresult
NS_GetFinalChannelURI(nsIChannel *channel, nsIURI **uri)
{
*uri = nullptr;
nsLoadFlags loadFlags = 0;
nsresult rv = channel->GetLoadFlags(&loadFlags);
NS_ENSURE_SUCCESS(rv, rv);

if (loadFlags & nsIChannel::LOAD_REPLACE) {
return channel->GetURI(uri);
nsCOMPtr<nsILoadInfo> loadInfo = channel->GetLoadInfo();
if (loadInfo) {
nsCOMPtr<nsIURI> resultPrincipalURI;
loadInfo->GetResultPrincipalURI(getter_AddRefs(resultPrincipalURI));
if (resultPrincipalURI) {
resultPrincipalURI.forget(uri);
return NS_OK;
}
}

return channel->GetOriginalURI(uri);
Expand Down
9 changes: 4 additions & 5 deletions netwerk/base/nsNetUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -781,11 +781,10 @@ nsresult NS_URIChainHasFlags(nsIURI *uri,
already_AddRefed<nsIURI> NS_GetInnermostURI(nsIURI *aURI);

/**
* Get the "final" URI for a channel. This is either the same as GetURI or
* GetOriginalURI, depending on whether this channel has
* nsIChanel::LOAD_REPLACE set. For channels without that flag set, the final
* URI is the original URI, while for ones with the flag the final URI is the
* channel URI.
* Get the "final" URI for a channel. This is either channel's load info
* resultPrincipalURI, if set, or GetOriginalURI. In most cases (but not all) load
* info resultPrincipalURI, if set, corresponds to URI of the channel if it's required
* to represent the actual principal for the channel.
*/
nsresult NS_GetFinalChannelURI(nsIChannel *channel, nsIURI **uri);

Expand Down
1 change: 1 addition & 0 deletions netwerk/ipc/NeckoChannelParams.ipdlh
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ struct LoadInfoArgs
PrincipalInfo triggeringPrincipalInfo;
OptionalPrincipalInfo principalToInheritInfo;
OptionalPrincipalInfo sandboxedLoadingPrincipalInfo;
OptionalURIParams resultPrincipalURI;
uint32_t securityFlags;
uint32_t contentPolicyType;
uint32_t tainting;
Expand Down
Loading

0 comments on commit b59d80d

Please sign in to comment.