functions-framework-nodejs icon indicating copy to clipboard operation
functions-framework-nodejs copied to clipboard

Functions Framework prevents developers from handling invalid params

Open inlined opened this issue 2 years ago • 9 comments

https://github.com/firebase/firebase-tools/issues/3891 seems to have a root cause in the functions framework.

To reproduce without Firebase:

const express = require('express');
const app = express();

app.get('/404', (req, res) => res.send('failed to work'));
app.get('**', (req, res) => res.send('working'));
app.use((err, req, res, next) => res.redirect('/404'));

module.exports.app = app;

if (require.main == module) {
  app.listen(8080);
}

If you run the file locally (no functions framework) and curl http://localhost:8080/%CO the user is redirected to http://localhost:8080/404.

If you deploy the function and CURL the repo (temporarily) at https://us-central1-inlined-junkdrawer.cloudfunctions.net/param-encoding/%C0, the functions framework rejects the request before it ever hits customer code with Bad Request

inlined avatar Dec 22 '21 16:12 inlined

CC @devlucem , this will be the new home for firebase-tools#3891

inlined avatar Dec 22 '21 16:12 inlined

Hi @inlined, thanks for the issue.

Can you please provide more details, including full repro steps in this issue (not linked issues)?

For example, the sample above uses Express and Express routing, not the Functions Framework.

grant avatar Dec 22 '21 20:12 grant

Thank you @inlined

@grant Here is the most basic code from cloud functions hello world

exports.helloWorld = (req, res) => {
  let message = req.query.message || req.body.message || 'Hello World!';
  res.status(200).send(message);
};

For this case, the unauthenticated invocations option has to be enabled as it is a publicly accessible website image

Using the trigger URL for any method should work and will work until you add %CO in the URL or any invalid param. These params should instead be handled in the hello world code and the function should work as normal as we are not decoding the params yet.

DevLucem avatar Dec 23 '21 06:12 DevLucem

Hi @devlucem, Thanks for your patience.

Here's the full repro:


index.js

exports.helloWorld = (req, res) => {
  let message = req.query.message || req.body.message || 'Hello World!';
  res.status(200).send(message);
};

package.json

{
  "main": "index.js",
  "dependencies": {
    "@google-cloud/functions-framework": "^3.0.0"
  }
}
npx @google-cloud/functions-framework --target helloWorld
Serving function...
Function: helloWorld
Signature type: http
URL: http://localhost:8080/
URIError: Failed to decode param '%CO'
    at decodeURIComponent (<anonymous>)

The full curl reveals the stacktrace:

URIError: Failed to decode param &#39;%CO&#39;
at decodeURIComponent (<anonymous>)
at decode_param (.../</anonymous>node_modules/express/lib/router/layer.js:172:12)
at Layer.match (.../</anonymous>node_modules/express/lib/router/layer.js:148:15)
at matchLayer (.../</anonymous>node_modules/express/lib/router/index.js:580:18)
at next (.../</anonymous>node_modules/express/lib/router/index.js:220:15)
at expressInit (.../</anonymous>node_modules/express/lib/middleware/init.js:40:5)
at Layer.handle [as handle_request] (.../</anonymous>node_modules/express/lib/router/layer.js:95:5)
at trim_prefix (.../</anonymous>node_modules/express/lib/router/index.js:323:13)
at .../</anonymous>node_modules/express/lib/router/index.js:284:7
at Function.process_params (.../</anonymous>node_modules/express/lib/router/index.js:341:12)

It is expected that the function wrapper handled the HTTP request to produce the (req) object.

There currently is no possible way to intercept exceptions like invalid URLs. If you'd like there to be a way, can you please file a GitHub issue in the main FF repo? https://github.com/GoogleCloudPlatform/functions-framework


In your application, if you really wanted to, you can workaround this issue by providing a custom middleware, although that is not officially supported. Here's a general handler method, and post for middleware:

  • https://stackoverflow.com/a/36125587/1233286
  • https://medium.com/google-cloud/express-routing-with-google-cloud-functions-36fb55885c68

I'm going to close this issue out, but if there are pending questions, please let us know. Hope this info is helpful.

grant avatar Mar 28 '22 23:03 grant

Thanks for your response @grant.

The middleware solution you offered is not supported and cannot be a workaround as the URL is checked before it gets to any cloud function. @inlined code shows the middleware sample.

DevLucem avatar Mar 29 '22 08:03 DevLucem

I also have tried a middleware solution and I can confirm that it doesn't work. The URIError is thrown before any middleware can catch it.

devgeni avatar Mar 30 '22 14:03 devgeni

Thanks.

What would you like the dev experience to be ideally? Expected behavior? Can you write that out?

I'm hearing you'd like to still invoke the function, but the server cannot accept a bad request. An error handler? Express sets up the router and processes params a bit out of our control.

grant avatar Mar 30 '22 15:03 grant

I expected for middleware workaround to solve this problem and honestly I can not come up with any idea of how this should behave. It looks like this is in the production code of thousands of devs. So we just can't intercept this exception on any level?

devgeni avatar Mar 31 '22 12:03 devgeni

To solve this, I think we'd need to support a lifecycle hook to handle BAD_REQUESTs to the function.

I wrote my thoughts here: https://github.com/GoogleCloudPlatform/functions-framework/issues/56

Similar: https://stackoverflow.com/a/31576777

grant avatar Apr 15 '22 22:04 grant

Closing this as it's tracked upstream against the FF spec.

josephlewis42 avatar Aug 29 '23 15:08 josephlewis42