Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(recovery): 🩹 automatic mode #1223

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions add-on/src/lib/ipfs-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,9 @@ export function createRequestModifier (getState, dnslinkResolver, ipfsPathValida
// to public gateway.
if (!state.nodeActive && request.type === 'main_frame' && sameGateway(request.url, state.gwURL)) {
const publicUri = await ipfsPathValidator.resolveToPublicUrl(request.url, state.pubGwURLString)
return handleRedirection({
originUrl: request.url,
redirectUrl: `${dropSlash(runtimeRoot)}${recoveryPagePath}#${encodeURIComponent(publicUri)}`
})
const redirectUrl = state.automaticMode ? publicUri : `${dropSlash(runtimeRoot)}${recoveryPagePath}#${encodeURIComponent(publicUri)}`

return handleRedirection({ originUrl: request.url, redirectUrl })
}

// When Subdomain Proxy is enabled we normalize address bar requests made
Expand Down
17 changes: 13 additions & 4 deletions add-on/src/lib/redirect-handler/blockOrObserve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,10 +320,13 @@ export function addRuleToDynamicRuleSetGenerator (
const state = getState()
const redirectIsOrigin = originUrl === redirectUrl
const redirectIsLocal = isLocalHost(originUrl) && isLocalHost(redirectUrl)
const badOriginRedirect = originUrl.includes(state.gwURL.host) && !redirectUrl.includes('recovery')
const localOriginRedirect = isLocalHost(originUrl) && !isLocalHost(redirectUrl)
// if the redirect is to the gateway and the automatic mode is off, we would like to present the recovery page.
// in case automatic mode is on, we would like to redirect to the public gateway.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the status of automatic mode should impact/bypass the recovery page – we don't want to blindly redirect users to third-party server – see #1204 (comment)

// This rule is added temporarily otherwise it messes up the local redirects.
const badOriginRedirect = localOriginRedirect && !state.automaticMode
// We don't want to redirect to the same URL. Or to the gateway.
if (redirectIsOrigin || badOriginRedirect || redirectIsLocal
) {
if (redirectIsOrigin || badOriginRedirect || redirectIsLocal) {
return
}

Expand All @@ -339,10 +342,11 @@ export function addRuleToDynamicRuleSetGenerator (
savedRegexFilters.delete(regexFilter)
}

const rule = saveAndGenerateRule(regexFilter, regexSubstitution)
await browser.declarativeNetRequest.updateDynamicRules(
{
// We need to add the new rule.
addRules: [saveAndGenerateRule(regexFilter, regexSubstitution)],
addRules: [rule],
// We need to remove the old rules.
removeRuleIds
}
Expand All @@ -351,6 +355,11 @@ export function addRuleToDynamicRuleSetGenerator (
// refresh the tab to apply the new rule.
const tabs = await browser.tabs.query({ url: `${originUrl}*` })
await Promise.all(tabs.map(async tab => await browser.tabs.reload(tab.id)))
if (state.automaticMode && localOriginRedirect) {
// Removing the recently added rule as this will mess up the local browsing.
await browser.declarativeNetRequest.updateDynamicRules({ removeRuleIds: [rule.id] })
savedRegexFilters.delete(regexFilter)
}
}

setupListeners(async (): Promise<void> => await reconcileRulesAndRemoveOld(getState()))
Expand Down
43 changes: 42 additions & 1 deletion test/functional/lib/ipfs-request-gateway-redirect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ describe('modifyRequest.onBeforeRequest:', function () {
})
})

describe('Recovers Page if node is unreachable', function () {
describe('Recovers Page if node is unreachable and automatic mode is off', function () {
beforeEach(function () {
global.browser = browser
state.ipfsNodeType = 'external'
Expand All @@ -436,6 +436,7 @@ describe('modifyRequest.onBeforeRequest:', function () {
state.gwURL = new URL('http://localhost:8080')
state.pubGwURLString = 'https://ipfs.io'
state.pubGwURL = new URL('https://ipfs.io')
state.automaticMode = false
})
it('should present recovery page if node is offline and redirect is enabled', async function () {
expect(state.nodeActive).to.be.equal(false)
Expand Down Expand Up @@ -465,6 +466,46 @@ describe('modifyRequest.onBeforeRequest:', function () {
})
})

describe('Redirects the Page if node is unreachable and automatic mode is on', function () {
beforeEach(function () {
global.browser = browser
state.ipfsNodeType = 'external'
state.redirect = true
state.peerCount = -1 // this simulates Kubo RPC being offline
state.gwURLString = 'http://localhost:8080'
state.gwURL = new URL('http://localhost:8080')
state.pubGwURLString = 'https://ipfs.io'
state.pubGwURL = new URL('https://ipfs.io')
state.automaticMode = true
})
it('should present recovery page if node is offline and redirect is enabled', async function () {
expect(state.nodeActive).to.be.equal(false)
state.redirect = true
const request = url2request('https://localhost:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar')
expect((await modifyRequest.onBeforeRequest(request)).redirectUrl).to.equal('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar')
})
it('should present recovery page if node is offline and redirect is disabled', async function () {
expect(state.nodeActive).to.be.equal(false)
state.redirect = false
const request = url2request('https://localhost:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar')
expect((await modifyRequest.onBeforeRequest(request)).redirectUrl).to.equal('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar')
})
it('should not show recovery page if node is offline, redirect is enabled, but non-gateway URL failed to load from the same port', async function () {
// this covers https://github.com/ipfs/ipfs-companion/issues/1162 and https://twitter.com/unicomp21/status/1626244123102679041
state.redirect = true
expect(state.nodeActive).to.be.equal(false)
const request = url2request('https://localhost:8080/')
expect(await modifyRequest.onBeforeRequest(request)).to.equal(undefined)
})
it('should not show recovery page if extension is disabled', async function () {
// allows user to quickly avoid anything similar to https://github.com/ipfs/ipfs-companion/issues/1162
state.active = false
expect(state.nodeActive).to.be.equal(false)
const request = url2request('https://localhost:8080/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR/foo/bar')
expect(await modifyRequest.onBeforeRequest(request)).to.equal(undefined)
})
})

after(function () {
delete global.URL
delete global.browser
Expand Down