firebase / firebase-tools

The Firebase Command Line Tools
MIT License
4.01k stars 926 forks source link

HTTP functions req.params is prefixed with "/" #3209

Open dora1998 opened 3 years ago

dora1998 commented 3 years ago

[REQUIRED] Environment info

firebase-tools: 9.6.1

Platform: macOS

[REQUIRED] Test case

When you use Cloud Functions with Hosting rewrites, you can get the path from req.params[0]. This value is different between local emulator and production.

import * as functions from "firebase-functions";

export const checkReqParams = functions.https.onRequest((req, res) => {
  res.status(200).send(req.params);
});

https://github.com/dora1998/firebase-tools-req-params

[REQUIRED] Steps to reproduce

  1. Access to http://localhost:5000/foobar (using hosting rewrites)
  2. You'll see the response below.
{
  "0": "/foobar"
}
  1. However, the response changes when you run this function in production.
{
  "0": "foobar"
}

[REQUIRED] Expected behavior

req.params[0] is not prefixed with / (same as production environment)

[REQUIRED] Actual behavior

req.params[0] is prefixed with / only in local emulator, not in production.

samtstern commented 3 years ago

@dora1998 thank you for the very clear explanation and sorry for the slow response! I'll take a look at this and submit a fix.

samtstern commented 3 years ago

So I was able to reproduce this and it actually has nothing to do with hosting rewrites. It seems that there's just a routing difference between the Functions emulator and Functions in production that affects req.params. I used your simple function with no rewrites, here's what I see in the emulators:

$ http http://localhost:5001/fir-dumpster/us-central1/checkReqParams
HTTP/1.1 200 OK
connection: keep-alive
content-length: 9
content-type: application/json; charset=utf-8
date: Fri, 19 Mar 2021 14:21:40 GMT
etag: W/"9-Q4XRrTcqwo1YnnY7oMqmiPMj3xw"
keep-alive: timeout=5
x-powered-by: Express

{
    "0": "/"
}

But in production:

$ http https://us-central1-fir-dumpster.cloudfunctions.net/checkReqParams/
HTTP/1.1 200 OK
Alt-Svc: h3-29=":443"; ma=2592000,h3-T051=":443"; ma=2592000,h3-Q050=":443"; ma=2592000,h3-Q046=":443"; ma=2592000,h3-Q043=":443"; ma=2592000,quic=":443"; ma=2592000; v="46,43"
Content-Length: 8
Content-Type: application/json; charset=utf-8
Date: Fri, 19 Mar 2021 14:27:34 GMT
Etag: W/"8-1hczpKomE5TQJKtSF5uA4u1x0nE"
Function-Execution-Id: p5265y5dcwb9
Server: Google Frontend
X-Cloud-Trace-Context: b75c3d40f4290d23a973f0729c832005
X-Powered-By: Express

{
    "0": ""
}

I'm wondering if there could be a difference in the versions of Express being used?

inlined commented 3 years ago

I believe this is because the emulator was written back in the node 6 days before the Functions Framework (or any other languages) were added. This was eventually changed in newer versions of Node to unify the routing logic across all languages. I'm not sure if supported versions of node differ in their behavior, but we can probably change the emulator to match production for node14 (if not match specific versions of Node)

yuchenshi commented 3 years ago

I've tracked this internally (b/183986140) and we welcome any Pull Requests to the emulator.