Skip to content

Commit

Permalink
Fix internal function URLs (#2984)
Browse files Browse the repository at this point in the history
  • Loading branch information
samtstern authored Jan 8, 2021
1 parent c75ab85 commit 3ed24ca
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
- Add warning when a developer is using yarn@2 PnP (#2198)
- Fixes incorrect URLs reported inside emulated HTTPS functions (#1862).
11 changes: 10 additions & 1 deletion src/emulator/functionsEmulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
5 changes: 1 addition & 4 deletions src/emulator/functionsEmulatorRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, () => {
Expand Down
38 changes: 35 additions & 3 deletions src/test/emulators/functionsEmulator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
),
};
Expand All @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions src/test/emulators/functionsEmulatorRuntime.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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(
Expand Down

0 comments on commit 3ed24ca

Please sign in to comment.