Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix HTTP routing issues #1332

Merged
merged 2 commits into from May 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog.txt
@@ -1,3 +1,5 @@
* Fixed bug in Firestore emulator where some FieldTransforms were sending back incorrect results.
* Fixed bug where read-only transactions would cause errors in the Firestore emulator.
* Fixed bug where collection group query listeners would cause errors in the Firestore emulator.
* Fixed bug where query parameters were not sent to HTTP functions.
* Fixed bug where some HTTP methods (like DELETE) were not allowed.
16 changes: 6 additions & 10 deletions src/emulator/functionsEmulator.ts
Expand Up @@ -27,7 +27,7 @@ import {
import { EmulatorRegistry } from "./registry";
import { EventEmitter } from "events";
import * as stream from "stream";
import { removePathSegments } from "./functionsEmulatorUtils";
import { trimFunctionPath } from "./functionsEmulatorUtils";
import { EmulatorLogger, Verbosity } from "./emulatorLogger";

const EVENT_INVOKE = "functions:invoke";
Expand Down Expand Up @@ -176,15 +176,13 @@ export class FunctionsEmulator implements EmulatorInstance {
`[functions] Runtime ready! Sending request! ${JSON.stringify(runtime.metadata)}`
);

/*
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.
*/
// 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.
const runtimeReq = http.request(
{
method,
path: "/" + removePathSegments(req.url, 3), // Remove the first 4 url paths like /X/X/X/a/b/c/
path: "/" + trimFunctionPath(req.url),
headers: req.headers,
socketPath: runtime.metadata.socketPath,
},
Expand Down Expand Up @@ -246,9 +244,7 @@ export class FunctionsEmulator implements EmulatorInstance {
// need to be registered first otherwise the HTTP functions consume
// all events.
hub.post(backgroundFunctionRoute, backgroundHandler);
hub.post(httpsFunctionRoutes, httpsHandler);
hub.get(httpsFunctionRoutes, httpsHandler);

hub.all(httpsFunctionRoutes, httpsHandler);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had no idea this existed, dope.

return hub;
}

Expand Down
3 changes: 1 addition & 2 deletions src/emulator/functionsEmulatorRuntime.ts
Expand Up @@ -549,8 +549,7 @@ async function ProcessHTTPS(frb: FunctionsRuntimeBundle, trigger: EmulatedTrigge
ephemeralServer.use(bodyParser.urlencoded({ extended: true }));
ephemeralServer.use(bodyParser.raw({ type: "*/*" }));

ephemeralServer.get("/*", handler);
ephemeralServer.post("/*", handler);
ephemeralServer.all("/*", handler);

const instance = ephemeralServer.listen(socketPath, () => {
new EmulatorLog("SYSTEM", "runtime-status", "ready", { socketPath }).log();
Expand Down
14 changes: 14 additions & 0 deletions src/emulator/functionsEmulatorUtils.ts
Expand Up @@ -2,6 +2,10 @@
Please be careful when adding require/imports to this file, it is pulled into functionsEmulatorRuntime
which is ran in a separate node process, so it is likely to have unintended side-effects for you.
*/

// Safe import because it's standard in Node
import * as url from "url";

const wildcardRegex = new RegExp("{[^/{}]*}");
const wildcardKeyRegex = new RegExp("^{(.+)}$");

Expand Down Expand Up @@ -62,3 +66,13 @@ export function removePathSegments(path: string, count: number): string {
.slice(count)
.join("/");
}

/**
* The full URL to an emulated function is /project/region/path(/subpath)?params but the function
* does not need to know about anything before the subpath.
*/
export function trimFunctionPath(path: string): string {
// Use the URL library to separate query params for later reconstruction.
const fakeURL = new url.URL(path, "https://example.com");
return removePathSegments(fakeURL.pathname, 3) + fakeURL.search;
}
21 changes: 21 additions & 0 deletions src/test/emulators/functionsEmulatorUtils.spec.ts
Expand Up @@ -2,6 +2,7 @@ import { expect } from "chai";
import {
extractParamsFromPath,
isValidWildcardMatch,
trimFunctionPath,
trimSlashes,
} from "../../emulator/functionsEmulatorUtils";

Expand Down Expand Up @@ -93,4 +94,24 @@ describe("FunctionsEmulatorUtils", () => {
expect(trimSlashes("///a////b//c/")).to.equal("a/b/c");
});
});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests 🙏

describe("trimFunctionPath", () => {
it("should remove the beginning of a function URL", () => {
expect(trimFunctionPath("/projectid/us-central1/functionPath")).to.equal("");
});
it("should not care about leading slashes", () => {
expect(trimFunctionPath("projectid/us-central1/functionPath")).to.equal("");
});
it("should preserve query parameters", () => {
expect(trimFunctionPath("/projectid/us-central1/functionPath?foo=bar")).to.equal("?foo=bar");
});
it("should preserve subpaths", () => {
expect(trimFunctionPath("/projectid/us-central1/functionPath/x/y")).to.equal("x/y");
});
it("should preserve subpaths with query parameters", () => {
expect(trimFunctionPath("/projectid/us-central1/functionPath/x/y?foo=bar")).to.equal(
"x/y?foo=bar"
);
});
});
});