diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index e1dbe3fde2..223b08bd98 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -1177,6 +1177,10 @@ export class DocWorkerApi { 'Content-Type': 'application/json', } }); + if (!ref.ok) { + res.status(ref.status).send(await ref.text()); + return; + } const states2: DocState[] = (await ref.json()).states; const left = states[0]; const right = states2[0]; diff --git a/app/server/lib/ITestingHooks.ts b/app/server/lib/ITestingHooks.ts index 3f5fa8d7d2..4a0f20a007 100644 --- a/app/server/lib/ITestingHooks.ts +++ b/app/server/lib/ITestingHooks.ts @@ -7,7 +7,6 @@ export interface ClientJsonMemoryLimits { } export interface ITestingHooks { - getOwnPort(): Promise; getPort(): Promise; setLoginSessionProfile(gristSidCookie: string, profile: UserProfile|null, org?: string): Promise; setServerVersion(version: string|null): Promise; diff --git a/app/server/lib/TestingHooks.ts b/app/server/lib/TestingHooks.ts index 0fb2fb8e3a..3c6956621c 100644 --- a/app/server/lib/TestingHooks.ts +++ b/app/server/lib/TestingHooks.ts @@ -68,11 +68,6 @@ export class TestingHooks implements ITestingHooks { private _workerServers: FlexServer[] ) {} - public async getOwnPort(): Promise { - log.info("TestingHooks.getOwnPort called"); - return this._server.getOwnPort(); - } - public async getPort(): Promise { log.info("TestingHooks.getPort called"); return this._port; diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index 2886d1d1fc..01ddb2a507 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -160,21 +160,23 @@ describe('DocApi', function () { testDocApi(); }); - describe("should work behind a reverse-proxy", async () => { - let proxy: TestServerReverseProxy; - - setup('behind-proxy', async () => { - proxy = new TestServerReverseProxy(); + describe('behind a reverse-proxy', function () { + async function setupServersWithProxy(suitename: string, overrideEnvConf?: NodeJS.ProcessEnv) { + const proxy = new TestServerReverseProxy(); const additionalEnvConfiguration = { ALLOWED_WEBHOOK_DOMAINS: `example.com,localhost:${webhooksTestPort}`, GRIST_DATA_DIR: dataDir, APP_HOME_URL: await proxy.getServerUrl(), GRIST_ORG_IN_PATH: 'true', GRIST_SINGLE_PORT: '0', + ...overrideEnvConf }; - home = await TestServer.startServer('home', tmpDir, suitename, additionalEnvConfiguration); - docs = await TestServer.startServer('docs', tmpDir, suitename, additionalEnvConfiguration, home.serverUrl); + const home = await TestServer.startServer('home', tmpDir, suitename, additionalEnvConfiguration); + const docs = await TestServer.startServer( + 'docs', tmpDir, suitename, additionalEnvConfiguration, home.serverUrl + ); proxy.requireFromOutsideHeader(); + await proxy.start(home, docs); homeUrl = serverUrl = await proxy.getServerUrl(); @@ -183,14 +185,68 @@ describe('DocApi', function () { Origin: serverUrl, ...TestServerReverseProxy.FROM_OUTSIDE_HEADER, }; - }); - after(async () => { + return {proxy, home, docs}; + } + + async function tearDown(proxy: TestServerReverseProxy, servers: TestServer[]) { proxy.stop(); + for (const server of servers) { + await server.stop(); + } await flushAllRedis(); + } + + let proxy: TestServerReverseProxy; + + describe('should run usual DocApi test', function () { + setup('behind-proxy-testdocs', async () => { + ({proxy, home, docs} = await setupServersWithProxy(suitename)); + }); + + after(() => tearDown(proxy, [home, docs])); + + testDocApi(); }); - testDocApi(); + async function testCompareDocs(proxy: TestServerReverseProxy, home: TestServer) { + const chimpy = makeConfig('chimpy'); + const userApiServerUrl = await proxy.getServerUrl(); + const chimpyApi = home.makeUserApi( + ORG_NAME, 'chimpy', { serverUrl: userApiServerUrl, headers: chimpy.headers as Record } + ); + const ws1 = (await chimpyApi.getOrgWorkspaces('current'))[0].id; + const docId1 = await chimpyApi.newDoc({name: 'testdoc1'}, ws1); + const docId2 = await chimpyApi.newDoc({name: 'testdoc2'}, ws1); + const doc1 = chimpyApi.getDocAPI(docId1); + + return doc1.compareDoc(docId2); + } + + describe('with APP_HOME_INTERNAL_URL', function () { + setup('behind-proxy-with-apphomeinternalurl', async () => { + // APP_HOME_INTERNAL_URL will be set by TestServer. + ({proxy, home, docs} = await setupServersWithProxy(suitename)); + }); + after(() => tearDown(proxy, [home, docs])); + + it('should succeed to compare docs', async function () { + const res = await testCompareDocs(proxy, home); + assert.exists(res); + }); + }); + + describe('without APP_HOME_INTERNAL_URL', function () { + setup('behind-proxy-without-apphomeinternalurl', async () => { + ({proxy, home, docs} = await setupServersWithProxy(suitename, {APP_HOME_INTERNAL_URL: ''})); + }); + after(() => tearDown(proxy, [home, docs])); + + it('should succeed to compare docs', async function () { + const promise = testCompareDocs(proxy, home); + await assert.isRejected(promise, /TestServerReverseProxy: called public URL/); + }); + }); }); describe("should work directly with a docworker", async () => { diff --git a/test/server/lib/helpers/TestServer.ts b/test/server/lib/helpers/TestServer.ts index 0de180bae4..8b791d02d7 100644 --- a/test/server/lib/helpers/TestServer.ts +++ b/test/server/lib/helpers/TestServer.ts @@ -6,7 +6,7 @@ import path from "path"; import * as fse from "fs-extra"; import * as testUtils from "test/server/testUtils"; import {UserAPIImpl} from "app/common/UserAPI"; -import {exitPromise} from "app/server/lib/serverUtils"; +import {exitPromise, getAvailablePort} from "app/server/lib/serverUtils"; import log from "app/server/lib/log"; import {delay} from "bluebird"; import fetch from "node-fetch"; @@ -44,9 +44,9 @@ export class TestServer { } public get proxiedServer() { return this._proxiedServer; } + private _serverUrl: string; private _server: ChildProcess; private _exitPromise: Promise; - private _serverUrl: string; private _proxiedServer: boolean = false; private readonly _defaultEnv; @@ -56,9 +56,6 @@ export class TestServer { GRIST_INST_DIR: this.rootDir, GRIST_DATA_DIR: path.join(this.rootDir, "data"), GRIST_SERVERS: this._serverTypes, - // with port '0' no need to hard code a port number (we can use testing hooks to find out what - // port server is listening on). - GRIST_PORT: '0', GRIST_DISABLE_S3: 'true', REDIS_URL: process.env.TEST_REDIS_URL, GRIST_TRIGGER_WAIT_DELAY: '100', @@ -80,10 +77,16 @@ export class TestServer { // Unix socket paths typically can't be longer than this. Who knew. Make the error obvious. throw new Error(`Path of testingSocket too long: ${this.testingSocket.length} (${this.testingSocket})`); } - const env = { - APP_HOME_URL: _homeUrl, - APP_HOME_INTERNAL_URL: _homeUrl, + + const port = await getAvailablePort(); + this._serverUrl = `http://localhost:${port}`; + const homeUrl = _homeUrl ?? (this._serverTypes.includes('home') ? this._serverUrl : undefined); + + const env: NodeJS.ProcessEnv = { + APP_HOME_URL: homeUrl, + APP_HOME_INTERNAL_URL: homeUrl, GRIST_TESTING_SOCKET: this.testingSocket, + GRIST_PORT: String(port), ...this._defaultEnv, ...customEnv }; @@ -138,8 +141,6 @@ export class TestServer { // create testing hooks and get own port this.testingHooks = await connectTestingHooks(this.testingSocket); - const port: number = await this.testingHooks.getOwnPort(); - this._serverUrl = `http://localhost:${port}`; // wait for check return (await fetch(`${this._serverUrl}/status/hooks`, {timeout: 1000})).ok; @@ -249,17 +250,20 @@ export class TestServerReverseProxy { if (this.stopped) { return; } - log.info("Stopping node TestServerProxy"); + log.info("Stopping node TestServerReverseProxy"); this._server.close(); } + // Inspired from: https://stackoverflow.com/a/10435819 private _getRequestHandlerFor(server: TestServer) { const serverUrl = new URL(server.serverUrl); return (oreq: express.Request, ores: express.Response) => { + log.debug(`[proxy] Requesting (method=${oreq.method}): ${new URL(oreq.url, serverUrl).href}`); + if (this._requireFromOutsideHeader && !isAffirmative(oreq.get("X-FROM-OUTSIDE"))) { - console.error('TestServerReverseProxy: called public URL from internal'); - return ores.json({error: "TestServerProxy: called public URL from internal "}).status(403); + log.error('TestServerReverseProxy: called public URL from internal'); + return ores.status(403).json({error: "TestServerReverseProxy: called public URL from internal "}); } const options = { @@ -270,8 +274,6 @@ export class TestServerReverseProxy { headers: oreq.headers, }; - log.debug(`[proxy] Requesting (method=${oreq.method}): ${new URL(oreq.url, serverUrl).href}`); - const creq = http .request(options, pres => { log.debug('[proxy] Received response for ' + oreq.url); @@ -299,7 +301,7 @@ export class TestServerReverseProxy { }) .on('error', e => { // we got an error - console.log(e.message); + log.info('Error caught by TestServerReverseProxy: %s', e.message); try { // attempt to set error message and http status ores.writeHead(500);