Skip to content

Commit

Permalink
fix: Unexpected "origin" value detected (value: "{"origin":"null"}")
Browse files Browse the repository at this point in the history
* the error could occur in some edge cases, like replying to the email with embedded images and enabled "Block non 'API entry point'-based network requests" feature (see #312 for details)
  • Loading branch information
vladimiry committed Nov 10, 2020
1 parent 197e5c8 commit 34e1da3
Showing 1 changed file with 54 additions and 26 deletions.
80 changes: 54 additions & 26 deletions src/electron-main/web-request/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,19 @@ const resolveFakeOrigin = (requestDetails: DeepReadonly<OnBeforeSendHeadersListe
return parseUrlOriginWithNullishCheck(requestDetails.url);
};

const isProtonEmbeddedUrl = (() => {
// https://github.com/ProtonMail/proton-shared/blob/84c149ebd0419e13e9a1504404a1d1803c53500c/lib/helpers/image.ts#L171
const nonSchemaLikePrefix = "cid:";

return (url: string): boolean => {
return (
url.startsWith(nonSchemaLikePrefix)
&&
url.substr(nonSchemaLikePrefix.length, 2) !== "//" // preventing "cid://"-like url schema use
);
};
})();

export function initWebRequestListenersByAccount(
ctx: DeepReadonly<Context>,
{
Expand Down Expand Up @@ -81,6 +94,8 @@ export function initWebRequestListenersByAccount(
{urls: []},
(details, callback) => {
const {url} = details;
const allowRequest = (): void => callback({});
const banRequest = (): void => callback({cancel: true});

if (
// feature enabled
Expand All @@ -92,6 +107,11 @@ export function initWebRequestListenersByAccount(
// only image resources
String(details.resourceType).toLowerCase() === "image"
&&
// TODO consider proxyfying only images with http/https schemes
// WARN: should be called before consequent "parseUrlOriginWithNullishCheck" call
// embedded/"cid:"-prefixed urls should not be processed/proxyfied (origin resolving for such urls returns "null")
!isProtonEmbeddedUrl(url)
&&
// resources served from "allowed origins" should not be proxified as those
// are not external (proton's static resource & API, devtools, etc)
!allowedOrigins.includes(
Expand Down Expand Up @@ -121,36 +141,44 @@ export function initWebRequestListenersByAccount(
return;
}

const additionAllowedOrigin = requestProxyCache.get(details)?.additionAllowedOrigin;
if (!blockNonEntryUrlBasedRequests) {
allowRequest();
return;
}

if (isProtonEmbeddedUrl(url)) {
// embedded/"cid:"-prefixed urls get silently blocked (origin resolving for such urls returns "null")
banRequest();
return;
}

const bannedUrlAccessMsg: null | string = blockNonEntryUrlBasedRequests
? buildUrlOriginsFailedMsgTester([
...allowedOrigins,
...(
additionAllowedOrigin
? [additionAllowedOrigin]
: []
),
])(url)
: null;

if (typeof bannedUrlAccessMsg === "string") {
setTimeout(() => { // can be asynchronous (speeds up callback resolving)
const message
= `Access to the "${details.resourceType}" resource with "${url}" URL has been forbidden. ${bannedUrlAccessMsg}`;
logger.error(message);
IPC_MAIN_API_NOTIFICATION$.next(
IPC_MAIN_API_NOTIFICATION_ACTIONS.ErrorMessage({message}),
);
});

requestProxyCache.remove(details);

callback({cancel: true});
const additionAllowedOrigin = requestProxyCache.get(details)?.additionAllowedOrigin;
const bannedUrlAccessMsg: null | string = buildUrlOriginsFailedMsgTester([
...allowedOrigins,
...(
additionAllowedOrigin
? [additionAllowedOrigin]
: []
),
])(url);

if (typeof bannedUrlAccessMsg !== "string") {
allowRequest();
return;
}

callback({});
setTimeout(() => { // can be asynchronous (speeds up callback resolving)
const message
= `Access to the "${details.resourceType}" resource with "${url}" URL has been forbidden. ${bannedUrlAccessMsg}`;
logger.error(message);
IPC_MAIN_API_NOTIFICATION$.next(
IPC_MAIN_API_NOTIFICATION_ACTIONS.ErrorMessage({message}),
);
});

requestProxyCache.remove(details);

banRequest();
},
);

Expand Down

0 comments on commit 34e1da3

Please sign in to comment.