Test APP_HOME_INTERNAL_URL

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];

app/server/lib/ITestingHooks.ts

@@ -7,7 +7,6 @@ export interface ClientJsonMemoryLimits {
 }
 
 export interface ITestingHooks {
-  getOwnPort(): Promise<number>;
   getPort(): Promise<number>;
   setLoginSessionProfile(gristSidCookie: string, profile: UserProfile|null, org?: string): Promise<void>;
   setServerVersion(version: string|null): Promise<void>;

app/server/lib/TestingHooks.ts

@@ -68,11 +68,6 @@ export class TestingHooks implements ITestingHooks {
     private _workerServers: FlexServer[]
   ) {}
 
-  public async getOwnPort(): Promise<number> {
-    log.info("TestingHooks.getOwnPort called");
-    return this._server.getOwnPort();
-  }
-
   public async getPort(): Promise<number> {
     log.info("TestingHooks.getPort called");
     return this._port;

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,70 @@ 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) {
+        // Pass kiwi's headers as it contains both Authorization and Origin headers
+        // if run behind a proxy, so we can ensure that the Origin header check is not made.
+        const chimpy = makeConfig('chimpy');
+        const userApiServerUrl = await proxy.getServerUrl();
+        const chimpyApi = home.makeUserApi(
+          ORG_NAME, 'chimpy', { serverUrl: userApiServerUrl, headers: chimpy.headers as Record<string, string> }
+        );
+        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.isDefined(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 () => {

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,12 @@ export class TestServer {
   }
   public get proxiedServer() { return this._proxiedServer; }
 
+  private get _serverUrl() {
+    return `http://localhost:${this._port}`;
+  }
   private _server: ChildProcess;
   private _exitPromise: Promise<number | string>;
-  private _serverUrl: string;
+  private _port: number;
   private _proxiedServer: boolean = false;
 
   private readonly _defaultEnv;
@@ -56,9 +59,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 +80,13 @@ 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,
+    this._port = await getAvailablePort();
+    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(this._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,19 @@ export class TestServerReverseProxy {
     if (this.stopped) {
       return;
     }
-    log.info("Stopping node TestServerProxy");
+    log.info("Stopping node TestServerReverseProxy");
     this._server.close();
   }
 
   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 +273,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 +300,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);