Repository: firebase/firebase-tools (ts)

Issue:
Request URLs are prefixed with the function "path"
<!-- DO NOT DELETE 
validate_template=true
template_path=.github/ISSUE_TEMPLATE/bug_report.md
-->

<!--
Thank you for contributing to the Firebase community!

Think you found a bug?
=======================
Yeah, we're definitely not perfect! Please use this template and include a minimal repro when opening the issue. If you know how to solve the issue, please create a Pull Request, and we'd be happy to review it!

Have a feature request?
========================
Great, we love hearing how we can improve our products! However, do not use this template to submit a feature request. Please submit your feature requests to: https://firebase.google.com/support/contact/bugs-features/

Have a usage question?
=======================
We get lots of those and we love helping you, but GitHub is not the best place for them and they will be closed. Please take a look at the guide first: https://firebase.google.com/docs/cli/

If the official documentation doesn't help, try asking through our official support channel: https://firebase.google.com/support/

Additional locations to check for solutions or assistance from the community:
- Stack Overflow: https://stackoverflow.com/
- Firebase Slack Community: https://firebase.community/

*Please avoid duplicate posting across multiple channels!*
-->

### [REQUIRED] Environment info

<!-- What version of the Firebase CLI (`firebase-tools`) are you using? Note that your issue may already be fixed in the latest versions. The latest version can be found at https://github.com/firebase/firebase-tools/releases -->

<!-- Output of `firebase --version` -->
**firebase-tools:** `7.9.0`

<!-- e.g. macOS, Windows, Ubuntu -->
**Platform:** Linux (Fedora 29)

### [REQUIRED] Test case

Request URLs are used in the SSR (angular-universal) for navigation it seems that there is a breaking change in how they are handled by function emulator.

The last version I verified that doesn't have this issue is `firebase-tools@6.8.0`.
From what I identified the problem seems to be related to request URLs. If I log the values I see
```
app.get('*', (req: express.Request, res: express.Response) => {

  
  console.log(req.url, req.baseUrl, req.originalUrl);
  //  '/en/contact' '/web/us-central1/ssr' '/web/us-central1/ssr/en/contact'

  res.render('web-index', { req, res });
});

```
But in case of the older versions of `firebase-tools` let's say the version `6.8.0`, output for the same code will be `'/en/contact', '', '/en/contact'`.


<!-- Provide a minimal, complete, and verifiable example (http://stackoverflow.com/help/mcve) -->


### [REQUIRED] Steps to reproduce

Create a firebase function with minimal express setup and log the request object.

<!-- Provide the steps needed to reproduce the issue with the above test case. -->

### [REQUIRED] Expected behavior

URLs probably shouldn't be prefixed by the function base.

As a quick fix, I'm using replacing the prefix with request.baseUrl as a base should represent the prefix, but I'm not sure if it is the case in every possible scenario.
```
app.get('*', (req: express.Request, res: express.Response) => {

  req.originalUrl = req.originalUrl.replace(req.baseUrl, '');

  res.render('web-index', { req, res });

});
```

Another **fix** that may be _a nicer solution_ is to explicitly set the `url` string.
`res.render('web-index', { req, res, url: req.url });`


<!-- What is the expected behavior? -->

### [REQUIRED] Actual behavior

URLs are prefixed by the function base. Router inside the universal rendering doesn't recognize the routes correctly and renders the default page.

As already mentioned in #1279 it seems like a breaking change that should be eighter corrected or documented.

<!-- Run the command with --debug flag, and include the logs below. -->


Pull request:
Fix internal function URLs
<!--

Thank you for contributing to the Firebase community! Please fill out the form below.

Run the linter and test suite
==============================
Run `npm test` to make sure your changes compile properly and the tests all pass on your local machine. We've hooked up this repo with continuous integration to double check those things for you.

-->

### Description

Fixes #1862, behavior now matches production.

### Scenarios Tested

See the test case here:
https://github.com/firebase/firebase-tools/issues/1862#issuecomment-752537717

Added unit tests to verify the behavior.

### Sample Commands

N/A

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 969d12d3b..72a9219fb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,1 +1,2 @@
 - Add warning when a developer is using yarn@2 PnP (#2198)
+- Fixes incorrect URLs reported inside emulated HTTPS functions (#1862).
diff --git a/src/emulator/functionsEmulator.ts b/src/emulator/functionsEmulator.ts
index f4b1deff1..c0b0018c1 100644
--- a/src/emulator/functionsEmulator.ts
+++ b/src/emulator/functionsEmulator.ts
@@ -1064,13 +1064,22 @@ export class FunctionsEmulator implements EmulatorInstance {
       );
     }
 
+    // To match production behavior we need to drop the path prefix
+    // req.url = /:projectId/:region/:trigger_name/*
+    const url = new URL(`${req.protocol}://${req.hostname}${req.url}`);
+    const path = `${url.pathname}${url.search}`.replace(
+      `/${this.args.projectId}/us-central1/${triggerId}`,
+      ""
+    );
+
     // We do this instead of just 302'ing because many HTTP clients don't respect 302s so it may
     // cause unexpected situations - not to mention CORS troubles and this enables us to use
     // a socketPath (IPC socket) instead of consuming yet another port which is probably faster as well.
+    this.logger.log("DEBUG", `[functions] Got req.url=${req.url}, mapping to path=${path}`);
     const runtimeReq = http.request(
       {
         method,
-        path: req.url || "/",
+        path,
         headers: req.headers,
         socketPath: worker.lastArgs.frb.socketPath,
       },
diff --git a/src/emulator/functionsEmulatorRuntime.ts b/src/emulator/functionsEmulatorRuntime.ts
index 43e673b73..9ccd843ed 100644
--- a/src/emulator/functionsEmulatorRuntime.ts
+++ b/src/emulator/functionsEmulatorRuntime.ts
@@ -850,10 +850,7 @@ async function processHTTPS(frb: FunctionsRuntimeBundle, trigger: EmulatedTrigge
 
     functionRouter.all("*", handler);
 
-    ephemeralServer.use(
-      [`/${frb.projectId}/${frb.triggerId}`, `/${frb.projectId}/:region/${frb.triggerId}`],
-      functionRouter
-    );
+    ephemeralServer.use([`/`, `/*`], functionRouter);
 
     logDebug(`Attempting to listen to socketPath: ${socketPath}`);
     const instance = ephemeralServer.listen(socketPath, () => {


Test patch:
diff --git a/src/test/emulators/functionsEmulator.spec.ts b/src/test/emulators/functionsEmulator.spec.ts
index 973dbed27..71f41b4b4 100644
--- a/src/test/emulators/functionsEmulator.spec.ts
+++ b/src/test/emulators/functionsEmulator.spec.ts
@@ -203,13 +203,43 @@ describe("FunctionsEmulator-Hub", () => {
       });
   }).timeout(TIMEOUT_LONG);
 
-  it("should rewrite req.baseUrl to show /:project_id/:region/:trigger_id", async () => {
+  it("should return the correct url, baseUrl, originalUrl for the root route", async () => {
     useFunctions(() => {
       require("firebase-admin").initializeApp();
       return {
         function_id: require("firebase-functions").https.onRequest(
           (req: express.Request, res: express.Response) => {
-            res.json({ baseUrl: req.baseUrl });
+            res.json({
+              url: req.url,
+              baseUrl: req.baseUrl,
+              originalUrl: req.originalUrl,
+            });
+          }
+        ),
+      };
+    });
+
+    await supertest(functionsEmulator.createHubServer())
+      .get("/fake-project-id/us-central1/function_id")
+      .expect(200)
+      .then((res) => {
+        expect(res.body.url).to.eq("/");
+        expect(res.body.baseUrl).to.eq("");
+        expect(res.body.originalUrl).to.eq("/");
+      });
+  }).timeout(TIMEOUT_LONG);
+
+  it("should return the correct url, baseUrl, originalUrl for a subroute", async () => {
+    useFunctions(() => {
+      require("firebase-admin").initializeApp();
+      return {
+        function_id: require("firebase-functions").https.onRequest(
+          (req: express.Request, res: express.Response) => {
+            res.json({
+              url: req.url,
+              baseUrl: req.baseUrl,
+              originalUrl: req.originalUrl,
+            });
           }
         ),
       };
@@ -219,7 +249,9 @@ describe("FunctionsEmulator-Hub", () => {
       .get("/fake-project-id/us-central1/function_id/sub/route/a")
       .expect(200)
       .then((res) => {
-        expect(res.body.baseUrl).to.eq("/fake-project-id/us-central1/function_id");
+        expect(res.body.url).to.eq("/sub/route/a");
+        expect(res.body.baseUrl).to.eq("");
+        expect(res.body.originalUrl).to.eq("/sub/route/a");
       });
   }).timeout(TIMEOUT_LONG);
 
diff --git a/src/test/emulators/functionsEmulatorRuntime.spec.ts b/src/test/emulators/functionsEmulatorRuntime.spec.ts
index 8f6752d21..e313b00cb 100644
--- a/src/test/emulators/functionsEmulatorRuntime.spec.ts
+++ b/src/test/emulators/functionsEmulatorRuntime.spec.ts
@@ -57,7 +57,7 @@ function invokeRuntimeWithFunctions(
 async function callHTTPSFunction(
   worker: RuntimeWorker,
   frb: FunctionsRuntimeBundle,
-  options: { headers?: { [key: string]: string } } = {},
+  options: { path?: string; headers?: { [key: string]: string } } = {},
   requestData?: string
 ): Promise<string> {
   await worker.waitForSocketReady();
@@ -67,7 +67,7 @@ async function callHTTPSFunction(
   }
 
   const socketPath = worker.lastArgs.frb.socketPath;
-  const path = `/${frb.projectId}/us-central1/${frb.triggerId}`;
+  const path = options.path || "/";
 
   const res = await new Promise<IncomingMessage>((resolve, reject) => {
     const req = request(