diff --git a/add-on/src/lib/redirect-handler/blockOrObserve.ts b/add-on/src/lib/redirect-handler/blockOrObserve.ts index dcb31d107..72779daeb 100644 --- a/add-on/src/lib/redirect-handler/blockOrObserve.ts +++ b/add-on/src/lib/redirect-handler/blockOrObserve.ts @@ -317,6 +317,7 @@ export function addRuleToDynamicRuleSetGenerator ( getState: () => CompanionState): (input: redirectHandlerInput) => Promise { // returning a closure to avoid passing `getState` as an argument to `addRuleToDynamicRuleSet`. return async function ({ originUrl, redirectUrl }: redirectHandlerInput): Promise { + // update the rules so that the next request is handled correctly. const state = getState() const redirectIsOrigin = originUrl === redirectUrl const redirectIsLocal = isLocalHost(originUrl) && isLocalHost(redirectUrl) @@ -327,7 +328,11 @@ export function addRuleToDynamicRuleSetGenerator ( return } - // We need to construct the regex filter and substitution. + // first update 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.update(tab.id, { url: redirectUrl }))) + + // Then update the rule set for future, we need to construct the regex filter and substitution. const { regexSubstitution, regexFilter } = constructRegexFilter({ originUrl, redirectUrl }) const savedRule = savedRegexFilters.get(regexFilter) @@ -347,10 +352,6 @@ export function addRuleToDynamicRuleSetGenerator ( removeRuleIds } ) - - // 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))) } setupListeners(async (): Promise => await reconcileRulesAndRemoveOld(getState())) diff --git a/test/functional/lib/redirect-handler/blockOrObserve.test.ts b/test/functional/lib/redirect-handler/blockOrObserve.test.ts index 7a11911b9..d8e336843 100644 --- a/test/functional/lib/redirect-handler/blockOrObserve.test.ts +++ b/test/functional/lib/redirect-handler/blockOrObserve.test.ts @@ -1,3 +1,4 @@ +import { reset } from './../../../../node_modules/ansi-colors/types/index.d'; import {expect} from 'chai' import {before, describe, it} from 'mocha' import sinon from 'sinon' @@ -30,9 +31,58 @@ const dynamicRulesConditions = (regexFilter) => ({ ] }) +const TEST_TAB_ID = 1234 +const LAST_GROUP_REGEX = '((?:[^\\.]|$).*)$' + +/** + * Ensures that the tab is redirected to the given url on the first request. + * + * @param url + */ +function ensureTabRedirected (url): void { + expect(browserMock.tabs.query.called).to.be.true + expect(browserMock.tabs.update.called).to.be.true + expect(browserMock.tabs.update.lastCall.args).to.deep.equal([TEST_TAB_ID, { url }]) +} + +/** + * Ensures that the declarativeNetRequest API is called with the expected rule. + * @param expectedCondition + * @param regexSubstitution + */ +function ensureDeclrativeNetRequetRuleIsAdded ({ + addRuleLength = 1, + callIndex = 0, + expectedCondition, + regexSubstitution, + removedRulesIds = [], +}: { + addRuleLength?: number + callIndex?: number + expectedCondition: string + regexSubstitution: string + removedRulesIds?: number[] +}): void { + if (callIndex < 0) { + callIndex = browserMock.declarativeNetRequest.updateDynamicRules.getCalls().length + callIndex + } + expect(browserMock.declarativeNetRequest.updateDynamicRules.called).to.be.true + const [{ addRules, removeRuleIds }] = browserMock.declarativeNetRequest.updateDynamicRules.getCall(callIndex).args + expect(removeRuleIds).to.deep.equal(removedRulesIds) + if (addRuleLength > 0) { + expect(addRules).to.have.lengthOf(addRuleLength) + const [{ id, priority, action, condition }] = addRules + expect(id).to.be.a('number') + expect(priority).to.equal(1) + expect(action).to.deep.equal({ type: 'redirect', redirect: { regexSubstitution } }) + expect(condition).to.deep.equal(dynamicRulesConditions(expectedCondition)) + } +} + describe('lib/redirect-handler/blockOrObserve', () => { before(function () { browserMock.runtime.id = 'testid' + browserMock.tabs.query.resolves([{id: TEST_TAB_ID}]) }) describe('isLocalHost', () => { @@ -68,6 +118,7 @@ describe('lib/redirect-handler/blockOrObserve', () => { sinonSandbox.restore() browserMock.tabs.query.resetHistory() browserMock.tabs.reload.resetHistory() + browserMock.tabs.update.resetHistory() browserMock.declarativeNetRequest = sinonSandbox.spy(new DeclarativeNetRequestMock()) // this cleans up the rules from the previous test stored in memory. await cleanupRules(true) @@ -83,6 +134,7 @@ describe('lib/redirect-handler/blockOrObserve', () => { // when redirectUrl is different from originUrl, but both are localhost. await addRuleToDynamicRuleSet({ originUrl: 'http://localhost:9001/foo', redirectUrl: 'http://localhost:9001/bar' }) expect(browserMock.declarativeNetRequest.updateDynamicRules.called).to.be.false + expect (browserMock.tabs.query.called).to.be.false }) it('Should allow pages to be recovered', async () => { @@ -91,7 +143,11 @@ describe('lib/redirect-handler/blockOrObserve', () => { originUrl: 'http://localhost:8080', redirectUrl: 'chrome-extension://some-path/dist/recover/recovery.html' }) - expect(browserMock.declarativeNetRequest.updateDynamicRules.called).to.be.true + ensureTabRedirected('chrome-extension://some-path/dist/recover/recovery.html') + ensureDeclrativeNetRequetRuleIsAdded({ + expectedCondition: `^https?\\:\\/\\/localhost\\:8080${LAST_GROUP_REGEX}`, + regexSubstitution: 'chrome-extension://some-path/dist/recover/recovery.html\\1', + }) }) it('Should add redirect rules for local gateway', async () => { @@ -99,15 +155,11 @@ describe('lib/redirect-handler/blockOrObserve', () => { originUrl: 'https://ipfs.io/ipns/en.wikipedia-on-ipfs.org', redirectUrl: 'http://localhost:8080/ipns/en.wikipedia-on-ipfs.org' }) - expect(browserMock.declarativeNetRequest.updateDynamicRules.called).to.be.true - const [{ addRules, removeRuleIds }] = browserMock.declarativeNetRequest.updateDynamicRules.firstCall.args - expect(removeRuleIds).to.deep.equal([]) - expect(addRules).to.have.lengthOf(1) - const [{ id, priority, action, condition }] = addRules - expect(id).to.be.a('number') - expect(priority).to.equal(1) - expect(action).to.deep.equal({ type: 'redirect', redirect: { "regexSubstitution": "http://localhost:8080\\1" } }) - expect(condition).to.deep.equal(dynamicRulesConditions('^https?\\:\\/\\/ipfs\\.io((?:[^\\.]|$).*)$')) + ensureTabRedirected('http://localhost:8080/ipns/en.wikipedia-on-ipfs.org') + ensureDeclrativeNetRequetRuleIsAdded({ + expectedCondition: `^https?\\:\\/\\/ipfs\\.io${LAST_GROUP_REGEX}`, + regexSubstitution: 'http://localhost:8080\\1' + }) }) it('Should add redirect for local gateway where originUrl is similar to redirectUrl', async () => { @@ -115,19 +167,11 @@ describe('lib/redirect-handler/blockOrObserve', () => { originUrl: 'https://docs.ipfs.tech', redirectUrl: 'http://localhost:8080/ipns/docs.ipfs.tech' }) - expect(browserMock.declarativeNetRequest.updateDynamicRules.called).to.be.true - const [{ addRules, removeRuleIds }] = browserMock.declarativeNetRequest.updateDynamicRules.firstCall.args - expect(removeRuleIds).to.deep.equal([]) - expect(addRules).to.have.lengthOf(1) - const [{ id, priority, action, condition }] = addRules - expect(id).to.be.a('number') - expect(priority).to.equal(1) - expect(action).to.deep.equal({ - type: 'redirect', redirect: { - "regexSubstitution": "http://localhost:8080/ipns/docs.ipfs.tech\\1" - } - }) - expect(condition).to.deep.equal(dynamicRulesConditions('^https?\\:\\/\\/docs\\.ipfs\\.tech((?:[^\\.]|$).*)$')) + ensureTabRedirected('http://localhost:8080/ipns/docs.ipfs.tech') + ensureDeclrativeNetRequetRuleIsAdded({ + expectedCondition: `^https?\\:\\/\\/docs\\.ipfs\\.tech${LAST_GROUP_REGEX}`, + regexSubstitution: 'http://localhost:8080/ipns/docs.ipfs.tech\\1' + }) }) it('Should add redirect for local gateway where originUrl is similar to redirectUrl and is not https', async () => { @@ -135,29 +179,68 @@ describe('lib/redirect-handler/blockOrObserve', () => { originUrl: 'http://docs.ipfs.tech', redirectUrl: 'http://localhost:8080/ipns/docs.ipfs.tech' }) - expect(browserMock.declarativeNetRequest.updateDynamicRules.called).to.be.true - const [{ addRules, removeRuleIds }] = browserMock.declarativeNetRequest.updateDynamicRules.firstCall.args - expect(removeRuleIds).to.deep.equal([]) - expect(addRules).to.have.lengthOf(1) - const [{ id, priority, action, condition }] = addRules - expect(id).to.be.a('number') - expect(priority).to.equal(1) - expect(action).to.deep.equal({ - type: 'redirect', redirect: { - "regexSubstitution": "http://localhost:8080/ipns/docs.ipfs.tech\\1" - } - }) - expect(condition).to.deep.equal(dynamicRulesConditions('^https?\\:\\/\\/docs\\.ipfs\\.tech((?:[^\\.]|$).*)$')) + ensureTabRedirected('http://localhost:8080/ipns/docs.ipfs.tech') + ensureDeclrativeNetRequetRuleIsAdded({ + expectedCondition: `^https?\\:\\/\\/docs\\.ipfs\\.tech${LAST_GROUP_REGEX}`, + regexSubstitution: 'http://localhost:8080/ipns/docs.ipfs.tech\\1' + }) }) - it('Should refresh the tab when redirect URL is added', async () => { - browserMock.tabs.query.resolves([{id: 1234}]) + it('Should remove the old rule when redirect changes for local gateway', async () => { + await addRuleToDynamicRuleSet({ + originUrl: 'http://docs.ipfs.tech', + redirectUrl: 'http://localhost:8080/ipns/docs.ipfs.tech' + }) + ensureTabRedirected('http://localhost:8080/ipns/docs.ipfs.tech') + ensureDeclrativeNetRequetRuleIsAdded({ + expectedCondition: `^https?\\:\\/\\/docs\\.ipfs\\.tech${LAST_GROUP_REGEX}`, + regexSubstitution: 'http://localhost:8080/ipns/docs.ipfs.tech\\1' + }) + const [{ addRules }] = browserMock.declarativeNetRequest.updateDynamicRules.firstCall.args + + await browserMock.declarativeNetRequest.updateDynamicRules.resetHistory() + // assuming the localhost changed or the redirectURL changed. + await addRuleToDynamicRuleSet({ + originUrl: 'http://docs.ipfs.tech', + redirectUrl: 'http://localhost:8081/ipns/docs.ipfs.tech' + }) + ensureTabRedirected('http://localhost:8081/ipns/docs.ipfs.tech') + ensureDeclrativeNetRequetRuleIsAdded({ + expectedCondition: `^https?\\:\\/\\/docs\\.ipfs\\.tech${LAST_GROUP_REGEX}`, + regexSubstitution: 'http://localhost:8081/ipns/docs.ipfs.tech\\1', + removedRulesIds: [addRules[0].id] + }) + }) + + it('Should remove the old rules when companion is no longer in active state', async () => { + // first redirect + const getRuleIdsAddedSoFar = () => browserMock.declarativeNetRequest.updateDynamicRules.getCalls().map(({args}) => args[0]?.addRules.map(rule => rule.id).flat()).flat() + + await addRuleToDynamicRuleSet({ + originUrl: 'http://docs.ipfs.tech', + redirectUrl: 'http://localhost:8080/ipns/docs.ipfs.tech' + }) + + await addRuleToDynamicRuleSet({ + originUrl: 'http://awesome.ipfs.io', + redirectUrl: 'http://localhost:8080/ipns/awesome.ipfs.io' + }) + + state.active = false await addRuleToDynamicRuleSet({ originUrl: 'https://ipfs.io/ipns/en.wikipedia-on-ipfs.org', redirectUrl: 'http://localhost:8080/ipns/en.wikipedia-on-ipfs.org' }) - sinon.assert.calledWith(browserMock.tabs.query, { url: 'https://ipfs.io/ipns/en.wikipedia-on-ipfs.org*' }) - sinon.assert.calledWith(browserMock.tabs.reload, 1234) + + ensureTabRedirected('http://localhost:8080/ipns/en.wikipedia-on-ipfs.org') + ensureDeclrativeNetRequetRuleIsAdded({ + addRuleLength: 0, + callIndex: -1, + expectedCondition: `^https?\\:\\/\\/ipfs\\.io${LAST_GROUP_REGEX}`, + regexSubstitution: 'http://localhost:8080\\1', + removedRulesIds: getRuleIdsAddedSoFar() + }) + }) }) })