Skip to content

Commit a9bdc30

Browse files
BijinDevcharlagpaw-hub
committed
Fix Unrecoverable state in login screen
Electron does not dispatch did-fail-load when we return a valid response e.g. 404 or 500 Fixed by throwing Error instead to setting status 404 which will dispatch did-fail-load should redirect to startPage whenever failed to load resource. Close: #9160 Co-authored-by: ivk <[email protected]> Co-authored-by: paw <[email protected]>
1 parent c52ee5d commit a9bdc30

File tree

4 files changed

+20
-20
lines changed

4 files changed

+20
-20
lines changed

src/common/desktop/ApplicationWindow.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -413,17 +413,14 @@ export class ApplicationWindow {
413413
})
414414
.on("did-fail-load", (evt, errorCode, errorDesc, validatedURL) => {
415415
log.debug(TAG, "failed to load resource: ", validatedURL, errorDesc)
416-
417-
if (errorDesc === "ERR_FILE_NOT_FOUND") {
418-
this.getInitialUrl({
419-
noAutoLogin: true,
416+
this.getInitialUrl({
417+
noAutoLogin: true,
418+
})
419+
.then((initialUrl) => {
420+
log.debug(TAG, "redirecting to start page...", initialUrl)
421+
return this._browserWindow.loadURL(initialUrl)
420422
})
421-
.then((initialUrl) => {
422-
log.debug(TAG, "redirecting to start page...", initialUrl)
423-
return this._browserWindow.loadURL(initialUrl)
424-
})
425-
.then(() => log.debug(TAG, "...redirected"))
426-
}
423+
.then(() => log.debug(TAG, "...redirected"))
427424
})
428425
// @ts-ignore
429426
.on("remote-require", (e) => e.preventDefault())

src/common/desktop/net/ProtocolProxy.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,9 @@ function handleAssetProtocol(session: Session, assetDir: string, pathModule: typ
8383
session.protocol.handle(ASSET_PROTOCOL, async (request: Request): Promise<Response> => {
8484
const fail = (msg: string) => {
8585
log.debug(TAG, msg)
86-
return new Response(null, { status: 404 })
86+
// electron does not dispatch did-fail-load when we return a valid response e.g. 404 or 500.
87+
// unexpected error at least get dispatched
88+
throw new Error(`assert protocol failure: ${msg}`)
8789
}
8890
// in node, new URL will normalize the path and remove /.. and /. elements.
8991
// this doesn't work in browsers, so the startsWith check below should stay just to be sure
@@ -98,7 +100,7 @@ function handleAssetProtocol(session: Session, assetDir: string, pathModule: typ
98100
return fail(`Invalid asset URL ${request.url} w/ pathname ${url.pathname} got resolved to ${filePath})`)
99101
} else {
100102
try {
101-
return fileFetch(filePath, fsModule)
103+
return await fileFetch(filePath, fsModule)
102104
} catch (e) {
103105
return fail(`failed to read asset at ${request.url}: ${e.message}`)
104106
}

test/tests/desktop/ApplicationWindowTest.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -378,22 +378,22 @@ o.spec("ApplicationWindow Test", function () {
378378
o(url.searchParams.get("theme")).equals(themeJson)
379379
})
380380

381-
o.test("redirect to start page after failing to load a page due to 404", async function () {
381+
o.test("redirect to start page after failing to load a page", async function () {
382382
const { wmMock, electronMock, electronLocalshortcutMock, themeFacade, remoteBridge } = standardMocks()
383383
const w = new ApplicationWindow(wmMock, desktopHtml, icon, electronMock, electronLocalshortcutMock, themeFacade, remoteBridge)
384384
const bwInstance = electronMock.BrowserWindow.mockedInstances[0]
385385
await bwInstance.__loadedUrl.promise
386386
bwInstance.__loadedUrl = defer()
387-
bwInstance.webContents.callbacks["did-fail-load"]({}, -6, "ERR_FILE_NOT_FOUND")
387+
bwInstance.webContents.callbacks["did-fail-load"]({}, -6)
388388
await bwInstance.__loadedUrl.promise
389389
o(bwInstance.loadURL.callCount).equals(2)
390390
const themeJson = JSON.stringify(await themeFacade.getCurrentThemeWithFallback())
391391
const url = new URL(bwInstance.loadURL.args[0])
392392
o(url.searchParams.get("noAutoLogin")).equals("true")
393393
o(url.searchParams.get("theme")).equals(themeJson)
394-
downcast(w._browserWindow.webContents).callbacks["did-fail-load"]({}, -6, "ERR_SOME_OTHER_ONE")
394+
downcast(w._browserWindow.webContents).callbacks["did-fail-load"]({}, -6)
395395
await delay(10)
396-
o(bwInstance.loadURL.callCount).equals(2)
396+
o(bwInstance.loadURL.callCount).equals(3)
397397
})
398398

399399
o.test("shortcut creation, linux", function () {

test/tests/desktop/net/ProtocolProxyTest.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { OutgoingHttpHeader } from "node:http"
44
import { func, matchers, object, verify, when } from "testdouble"
55
import { doHandleProtocols, handleProtocols } from "../../../../src/common/desktop/net/ProtocolProxy.js"
66
import { utf8Uint8ArrayToString } from "@tutao/tutanota-utils"
7+
import { assertThrows } from "@tutao/tutanota-test-utils"
78

89
o.spec("ProtocolProxy", function () {
910
let fetchMock
@@ -137,17 +138,17 @@ o.spec("ProtocolProxy", function () {
137138
when(protocol.handle("asset", captor.capture())).thenReturn(undefined)
138139
doHandleProtocols(ses, "/tutanota/assets", fetchMock, path, fs)
139140
const request = { url: "basset://app/noodles.txt" }
140-
const responseFromSubject = await captor.value(request)
141-
o(await responseFromSubject.status).deepEquals(404)
141+
const error = await assertThrows(Error, () => captor.value(request))
142+
o.check(error.message).equals("assert protocol failure: passed non-asset url to asset handler: basset://app/noodles.txt")
142143
})
143144

144145
o("rejects non-app hostname", async function () {
145146
const captor = matchers.captor()
146147
when(protocol.handle("asset", captor.capture())).thenReturn(undefined)
147148
doHandleProtocols(ses, "/tutanota/assets", fetchMock, path, fs)
148149
const request = { url: "asset://bop/noodles.txt" }
149-
const responseFromSubject = await captor.value(request)
150-
o(await responseFromSubject.status).deepEquals(404)
150+
const error = await assertThrows(Error, () => captor.value(request))
151+
o.check(error.message).equals("assert protocol failure: Invalid asset:// URL: asset://bop/noodles.txt")
151152
})
152153
})
153154
})

0 commit comments

Comments
 (0)