Bug 1844150 - Don't add moz-anno:favicon to local protocols. r=daisuke,sessionstore-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D184376
This commit is contained in:
Marco Bonardo 2023-07-25 16:37:14 +00:00
parent 624f7efa66
commit 93119159bf
9 changed files with 115 additions and 63 deletions

View file

@ -105,11 +105,9 @@ export class AboutReaderParent extends JSWindowActorParent {
uri,
iconUri => {
if (iconUri) {
iconUri =
lazy.PlacesUtils.favicons.getFaviconLinkForIcon(iconUri);
resolve({
url: message.data.url,
faviconUrl: iconUri.pathQueryRef.replace(/^favicon:/, ""),
faviconUrl: iconUri.spec,
});
} else {
resolve(null);

View file

@ -1843,13 +1843,24 @@ export var PlacesUIUtils = {
}
},
/**
* Generates a moz-anno:favicon: link for an icon URL, that will allow to
* fetch the icon from the local favicons cache, rather than from the network.
* If the icon URL is invalid, fallbacks to the default favicon URL.
*
* @param {string} icon The url of the icon to load from local cache.
* @returns {string} a "moz-anno:favicon:" prefixed URL, unless the original
* URL protocol refers to a local resource, then it will just pass-through
* unchanged.
*/
getImageURL(icon) {
let iconURL = icon;
// don't initiate a connection just to fetch a favicon (see bug 467828)
if (/^https?:/.test(iconURL)) {
iconURL = "moz-anno:favicon:" + iconURL;
}
return iconURL;
try {
return lazy.PlacesUtils.favicons.getFaviconLinkForIcon(
Services.io.newURI(icon)
).spec;
} catch (ex) {}
return lazy.PlacesUtils.favicons.defaultFavicon.spec;
},
/**

View file

@ -8,6 +8,7 @@ const { AppConstants } = ChromeUtils.importESModule(
"resource://gre/modules/AppConstants.sys.mjs"
);
ChromeUtils.defineESModuleGetters(this, {
PlacesUIUtils: "resource:///modules/PlacesUIUtils.sys.mjs",
SessionStore: "resource:///modules/sessionstore/SessionStore.sys.mjs",
});
@ -107,15 +108,11 @@ async function initTreeView() {
var entry = aTabData.entries[aTabData.index - 1] || {
url: "about:blank",
};
var iconURL = aTabData.image || null;
// don't initiate a connection just to fetch a favicon (see bug 462863)
if (/^https?:/.test(iconURL)) {
iconURL = "moz-anno:favicon:" + iconURL;
}
return {
label: entry.title || entry.url,
checked: true,
src: iconURL,
src: PlacesUIUtils.getImageURL(aTabData.image),
parent: winState,
};
});

View file

@ -604,13 +604,6 @@ TabWindow.prototype = {
}
},
directRequestProtocols: new Set([
"file",
"chrome",
"resource",
"about",
"data",
]),
onLinkIconAvailable(aBrowser, aIconURL) {
let tab = this.win.gBrowser.getTabForBrowser(aBrowser);
this.updateFavicon(tab, aIconURL);
@ -618,17 +611,13 @@ TabWindow.prototype = {
updateFavicon(aTab, aIconURL) {
let requestURL = null;
if (aIconURL) {
let shouldRequestFaviconURL = true;
try {
let urlObject = NetUtil.newURI(aIconURL);
shouldRequestFaviconURL = !this.directRequestProtocols.has(
urlObject.scheme
);
} catch (ex) {}
requestURL = shouldRequestFaviconURL
? "moz-anno:favicon:" + aIconURL
: aIconURL;
requestURL = PlacesUtils.favicons.getFaviconLinkForIcon(
Services.io.newURI(aIconURL)
).spec;
} catch (ex) {
requestURL = aIconURL;
}
}
let isDefaultFavicon = !requestURL;
getFaviconAsImage(

View file

@ -81,7 +81,7 @@ nsresult GetFramesInfoForContainer(imgIContainer* aContainer,
continue;
}
// Check if it's one of the sizes we care about.
auto end = std::end(gFaviconSizes);
const auto* end = std::end(gFaviconSizes);
const uint16_t* matchingSize =
std::find(std::begin(gFaviconSizes), end, nativeSize.width);
if (matchingSize != end) {
@ -381,12 +381,12 @@ nsFaviconService::ReplaceFaviconData(nsIURI* aFaviconURI,
return NS_ERROR_OUT_OF_MEMORY;
}
iconKey->created = PR_Now();
iconKey->created = now;
// If the cache contains unassociated icons, an expiry timer should already
// exist, otherwise there may be a timer left hanging around, so make sure we
// fire a new one.
int32_t unassociatedCount = mUnassociatedIcons.Count();
uint32_t unassociatedCount = mUnassociatedIcons.Count();
if (unassociatedCount == 1) {
mExpireUnassociatedIconsTimer->Cancel();
mExpireUnassociatedIconsTimer->InitWithCallback(
@ -505,8 +505,9 @@ nsFaviconService::ReplaceFaviconDataFromDataURL(
uint64_t available64;
rv = stream->Available(&available64);
NS_ENSURE_SUCCESS(rv, rv);
if (available64 == 0 || available64 > UINT32_MAX / sizeof(uint8_t))
if (available64 == 0 || available64 > UINT32_MAX / sizeof(uint8_t)) {
return NS_ERROR_FILE_TOO_BIG;
}
uint32_t available = (uint32_t)available64;
// Read all the decoded data.
@ -629,16 +630,37 @@ nsFaviconService::CopyFavicons(nsIURI* aFromPageURI, nsIURI* aToPageURI,
}
nsresult nsFaviconService::GetFaviconLinkForIcon(nsIURI* aFaviconURI,
nsIURI** aOutputURI) {
nsIURI** _retval) {
NS_ENSURE_ARG(aFaviconURI);
NS_ENSURE_ARG_POINTER(aOutputURI);
NS_ENSURE_ARG_POINTER(_retval);
nsAutoCString spec;
if (aFaviconURI) {
// List of protocols for which it doesn't make sense to generate a favicon
// uri since they can be directly loaded from disk or memory.
static constexpr nsLiteralCString sDirectRequestProtocols[] = {
// clang-format off
"about"_ns,
"chrome"_ns,
"data"_ns,
"file"_ns,
"moz-anno"_ns,
"resource"_ns,
// clang-format on
};
nsAutoCString iconURIScheme;
if (NS_SUCCEEDED(aFaviconURI->GetScheme(iconURIScheme)) &&
std::find(std::begin(sDirectRequestProtocols),
std::end(sDirectRequestProtocols),
iconURIScheme) != std::end(sDirectRequestProtocols)) {
// Just return the input URL.
*_retval = do_AddRef(aFaviconURI).take();
return NS_OK;
}
nsresult rv = aFaviconURI->GetSpec(spec);
NS_ENSURE_SUCCESS(rv, rv);
}
return GetFaviconLinkForIconString(spec, aOutputURI);
return GetFaviconLinkForIconString(spec, _retval);
}
// nsFaviconService::GetFaviconLinkForIconString
@ -760,7 +782,7 @@ nsresult nsFaviconService::OptimizeIconSizes(IconData& aIcon) {
}
nsresult nsFaviconService::GetFaviconDataAsync(
const nsCString& aFaviconURI, mozIStorageStatementCallback* aCallback) {
const nsCString& aFaviconSpec, mozIStorageStatementCallback* aCallback) {
MOZ_ASSERT(aCallback, "Doesn't make sense to call this without a callback");
nsCOMPtr<mozIStorageAsyncStatement> stmt = mDB->GetAsyncStatement(
@ -770,7 +792,7 @@ nsresult nsFaviconService::GetFaviconDataAsync(
"ORDER BY width DESC");
NS_ENSURE_STATE(stmt);
nsresult rv = URIBinder::Bind(stmt, "url"_ns, aFaviconURI);
nsresult rv = URIBinder::Bind(stmt, "url"_ns, aFaviconSpec);
NS_ENSURE_SUCCESS(rv, rv);
nsCOMPtr<mozIStoragePendingStatement> pendingStatement;
@ -795,11 +817,12 @@ nsFaviconService::PreferredSizeFromURI(nsIURI* aURI, uint16_t* _size) {
int32_t start = ref.RFind("size=");
if (start >= 0 && ref.Length() > static_cast<uint32_t>(start) + 5) {
nsDependentCSubstring size;
// This is safe regardless, since Rebind checks start is not over Length().
// This is safe regardless, since Rebind checks start is not over
// Length().
size.Rebind(ref, start + 5);
// Check if the string contains any non-digit.
auto begin = size.BeginReading(), end = size.EndReading();
for (auto ch = begin; ch < end; ++ch) {
for (const auto* ch = begin; ch < end; ++ch) {
if (*ch < '0' || *ch > '9') {
// Not a digit.
return NS_OK;

View file

@ -23,6 +23,7 @@
#include "nsTHashtable.h"
#include "nsToolkitCompsCID.h"
#include "nsURIHashKey.h"
#include "prtime.h"
// The target dimension in pixels for favicons we store, in reverse order.
// When adding/removing sizes from here, make sure to update the vector size.
@ -33,13 +34,12 @@ class mozIStorageStatementCallback;
class UnassociatedIconHashKey : public nsURIHashKey {
public:
explicit UnassociatedIconHashKey(const nsIURI* aURI) : nsURIHashKey(aURI) {}
UnassociatedIconHashKey(UnassociatedIconHashKey&& aOther)
explicit UnassociatedIconHashKey(const nsIURI* aURI)
: nsURIHashKey(aURI), created(PR_Now()) {}
UnassociatedIconHashKey(UnassociatedIconHashKey&& aOther) noexcept
: nsURIHashKey(std::move(aOther)),
iconData(std::move(aOther.iconData)),
created(std::move(aOther.created)) {
MOZ_ASSERT_UNREACHABLE("Do not call me!");
}
created(std::move(aOther.created)) {}
mozilla::places::IconData iconData;
PRTime created;
};
@ -75,7 +75,7 @@ class nsFaviconService final : public nsIFaviconService,
}
// addition to API for strings to prevent excessive parsing of URIs
nsresult GetFaviconLinkForIconString(const nsCString& aIcon,
nsresult GetFaviconLinkForIconString(const nsCString& aSpec,
nsIURI** aOutput);
nsresult OptimizeIconSizes(mozilla::places::IconData& aIcon);

View file

@ -12,14 +12,14 @@ add_task(async function () {
let data = readFileData(do_get_file("favicon-normal16.png"));
PlacesUtils.favicons.replaceFaviconData(
NetUtil.newURI(ICON16_URL),
Services.io.newURI(ICON16_URL),
data,
"image/png"
);
await setFaviconForPage(PAGE_URL, ICON16_URL);
data = readFileData(do_get_file("favicon-normal32.png"));
PlacesUtils.favicons.replaceFaviconData(
NetUtil.newURI(ICON32_URL),
Services.io.newURI(ICON32_URL),
data,
"image/png"
);
@ -29,44 +29,44 @@ add_task(async function () {
await compareFavicons(
PAGE_ICON_URL,
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON32_URL)),
"Not specifying a ref should return the bigger icon"
);
// Fake window object.
let win = { devicePixelRatio: 1.0 };
await compareFavicons(
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 16),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON16_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON16_URL)),
"Size=16 should return the 16px icon"
);
await compareFavicons(
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 32),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON32_URL)),
"Size=32 should return the 32px icon"
);
await compareFavicons(
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 33),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON32_URL)),
"Size=33 should return the 32px icon"
);
await compareFavicons(
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 17),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON32_URL)),
"Size=17 should return the 32px icon"
);
await compareFavicons(
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 1),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON16_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON16_URL)),
"Size=1 should return the 16px icon"
);
await compareFavicons(
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 0),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON32_URL)),
"Size=0 should return the bigger icon"
);
await compareFavicons(
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, -1),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON32_URL)),
"Invalid size should return the bigger icon"
);
@ -76,31 +76,31 @@ add_task(async function () {
await setFaviconForPage(PAGE_URL + "#other§=12", ICON32_URL, false);
await compareFavicons(
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL + "#other§=12", 16),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON16_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON16_URL)),
"Pre-existing refs should be retained"
);
await compareFavicons(
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL + "#other§=12", 32),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON32_URL)),
"Pre-existing refs should be retained"
);
// If the ref-ed url is unknown, should still try to fetch icon for the unref-ed url.
await compareFavicons(
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL + "#randomstuff", 32),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON32_URL)),
"Non-existing refs should be ignored"
);
win = { devicePixelRatio: 1.1 };
await compareFavicons(
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 16),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON32_URL)),
"Size=16 with HIDPI should return the 32px icon"
);
await compareFavicons(
PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 32),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON32_URL)),
"Size=32 with HIDPI should return the 32px icon"
);
@ -108,7 +108,7 @@ add_task(async function () {
PlacesUtils.favicons.setDefaultIconURIPreferredSize(16);
await compareFavicons(
PAGE_ICON_URL,
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON16_URL)),
PlacesUtils.favicons.getFaviconLinkForIcon(Services.io.newURI(ICON16_URL)),
"Not specifying a ref should return the set default size icon"
);
});

View file

@ -0,0 +1,33 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
/**
* Test getFaviconLinkForIcon API.
*/
add_task(async function test_basic() {
// Check these protocols are pass-through.
for (let protocol of ["http://", "https://"]) {
let url = PlacesUtils.favicons.getFaviconLinkForIcon(
Services.io.newURI(protocol + "test/test.png")
).spec;
Assert.equal(url, "moz-anno:favicon:" + protocol + "test/test.png");
}
});
add_task(async function test_directRequestProtocols() {
// Check these protocols are pass-through.
for (let protocol of [
"about:",
"chrome://",
"data:",
"file:///",
"moz-anno:",
"resource://",
]) {
let url = PlacesUtils.favicons.getFaviconLinkForIcon(
Services.io.newURI(protocol + "test/test.png")
).spec;
Assert.equal(url, protocol + "test/test.png");
}
});

View file

@ -33,6 +33,7 @@ support-files =
[test_favicons_conversions.js]
[test_favicons_protocols_ref.js]
[test_getFaviconDataForPage.js]
[test_getFaviconLinkForIcon.js]
[test_getFaviconURLForPage.js]
[test_heavy_favicon.js]
[test_incremental_vacuum.js]