netlify / cli

Netlify Command Line Interface
http://cli.netlify.com
MIT License
1.58k stars 358 forks source link

Feature Request: Allow passing arbitrary data to functions:invoke payload flag #980

Open dansteren opened 4 years ago

dansteren commented 4 years ago

- Do you want to request a feature or report a bug?

Feature Request. It might be considered a bug, but the documentation only makes mention of JSON payloads, so I'm going to assume non-JSON payloads aren't officially supported.

- What is the current behavior?

Calling netlify functions:invoke and passing something other than JSON to the --payload argument results in the payload not being added to the request body.

For example, calling netlify functions:invoke myFunction --payload 'myPayload.txt' where myPayload.txt contains the string thisLineIsExecutedNotRead results in the following error:

ReferenceError: thisLineIsExecutedNotRead is not defined
    at Object.<anonymous> (/Users/dan/repos/dansteren/tribute/test/mypayload.txt:1:1)
    at Module._compile (internal/modules/cjs/loader.js:1200:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1220:10)
    at Module.load (internal/modules/cjs/loader.js:1049:32)
    at Function.Module._load (internal/modules/cjs/loader.js:937:14)
    at Module.require (internal/modules/cjs/loader.js:1089:19)
    at require (internal/modules/cjs/helpers.js:73:18)
    at processPayloadFromFlag (/usr/local/lib/node_modules/netlify-cli/src/commands/functions/invoke.js:156:19)
    at FunctionsInvokeCommand.run (/usr/local/lib/node_modules/netlify-cli/src/commands/functions/invoke.js:109:21)
    at async FunctionsInvokeCommand._run (/usr/local/lib/node_modules/netlify-cli/node_modules/@oclif/command/lib/command.js:43:20)
Function invocation failed: TypeError: Cannot read property 'actions' of undefined

This makes it impossible to use netlify functions:invoke if you need to pass a generic string.

- If the current behavior is a bug, please provide the steps to reproduce.

  1. Create a function that you can hit with netlify functions:invoke
  2. Attempt to pass a string payload to the function with this command netlify functions:invoke myFunction --payload 'hello world'
  3. Note that the string function doesn't get passed in the body of your request
  4. Attempt to pass a string payload as a file reference: netlify functions:invoke myFunction --payload './myPayload.txt (where myPayload.txt contains a string of random characters, not a JSON string)
  5. Note that the CLI throws an error

- What is the expected behavior?

Looking at the code I'm seeing that the method processPayloadFromFlag in invoke.js expects the payload to be a JSON string. I think that assumption should not be made. I'm also seeing this on line 156

payload = require(payloadpath) // there is code execution potential here

I think rather than trying to require the file (which works for JSON and JS) it should just read the file. This wouldn't evaluate JS anymore, but would be better for JSON, XML, or anything other than JS really. This way the user could specify their body in whatever format they like and know that the contents of their file is the contents of the body of their request.

If the file was read rather than required then it also necessitate a change like this where the function is invoked:

+     const payload = processPayloadFromFlag(flags.payload)
-     body = Object.assign({}, body, payload)
+     body = Object.assign({}, body, payload) // Not quite sure what to do with this line since I don't understand what `body` is before this.
+ 
+     // fetch
+     fetch(`http://localhost:${port}/.netlify/functions/${functionToTrigger}` + formatQstring(flags.querystring), {
+       method: 'post',
+       headers,
-       body: JSON.stringify(body),
+       body: payload,
+     })

Note: If the expected behavior is that .js files should be required then the file extension could be used to differentiate between js and non-js files. That way .js files could be required, and .json, .txt, .xml, etc files could be read instead.

- Local Environment Information

────────────────────┐ Environment Info │ ────────────────────┘

System: OS: macOS 10.15.4 CPU: (8) x64 Intel(R) Core(TM) i7-7820HQ CPU @ 2.90GHz Binaries: Node: 14.4.0 - /usr/local/bin/node Yarn: 1.22.4 - /usr/local/bin/yarn npm: 6.14.4 - /usr/local/bin/npm Browsers: Chrome: 83.0.4103.116 Firefox: 72.0.1 Safari: 13.1 npmGlobalPackages: netlify-cli: 2.54.0

netlify-cli/2.54.0 darwin-x64 node-v14.4.0

simonmulser commented 3 years ago

Are PRs welcome for this one? If yes, high level description of how this should be done would be nice.

erezrokah commented 3 years ago

Thanks @simonmulser - a PR for this would be great. The payload flag is parsed here: https://github.com/netlify/cli/blob/66fd64c0da3756ec6f91ed6c8f7f5f00883a882a/src/commands/functions/invoke.js#L122

The reason we enforce a JSON payload at the moment is because it is required for event triggered functions (e.g. identity-signup): https://github.com/netlify/cli/blob/66fd64c0da3756ec6f91ed6c8f7f5f00883a882a/src/commands/functions/invoke.js#L63

We could enforce a JSON payload only when a function matches an event triggered function and not for all functions. Does that makes sense?