Add env vars to use raw body and set max json size #195
Conversation
This change adds the use of 2 environment variables allowing users to receive the request body as a buffer (the 'raw' body) as well as the ability to set the maximum JSON body size. The raw body is required when processing Stripe webhooks The max JSON body size is defaulted to a low limit from body-parser which has caused issues with users in the past. Signed-off-by: Burton Rheutan <[email protected]>
| app.use(bodyParser.json()); | ||
| app.use(bodyParser.raw()); | ||
| app.use(bodyParser.text({ type : "text/*" })); | ||
| if (process.env.RAW_BODY === 'true') { |
There was a problem hiding this comment.
Hey @burtonr
I'd suggest to keep support of json and text parsers even when RAW_BODY is true.
To keep completely old behavior I suggest inverting the variable name something like NORAW_BODY and enable raw parser by default.
jsonLimit can be also set only if MAX_JSON_SIZE is specified
There was a problem hiding this comment.
I'd suggest to keep support of json and text parsers even when RAW_BODY is true.
I don't see the point in that change, that's the behaviour we have today?
There was a problem hiding this comment.
If I understand this correctly RAW_BODY should just enable/disable raw parser, is not it?
If it is set to true the app will have no JSON and TEXT parsers
There was a problem hiding this comment.
The use-case was that users need to disable JSON parsing and only access a RAW body. The docs PR I hope helps with this, but I'd like @burtonr to explain the use-case better there. https://docs.openfaas.com/cli/templates/#nodejs-12-node12-of-watchdog-template
There was a problem hiding this comment.
RAW_BODY confuses me since it was TLDR. maybe ONLY_RAW name is better?
There was a problem hiding this comment.
That's right, but it's important to understand that the bodyParser.raw() only enables the raw body for the content type that you pass in. So, this change makes it so that when a user sets the RAW_BODY variable to true, the raw body will be available for all content types.
The idea is that a user would define a very specific function that requires the raw body, where other functions will not have this enabled and have the JSON body available as normal.
Description
This change adds the use of 2 environment variables:
RAW_BODYandMAX_JSON_SIZE, allowing users to receive therequest body as a buffer (the 'raw' body) as well as the ability to set the maximum
JSON body size.
The raw body is required when processing Stripe webhooks
The max JSON body size is defaulted to a low limit from body-parser which has caused
issues with users in the past.
Signed-off-by: Burton Rheutan [email protected]
Motivation and Context
There was some discussion in the issue linked below. I've decided to implement the change in this was so that it was a more overall change rather than adding it to a specific verb (
POST) to avoid surprising results when a user sets theRAW_BODYto true, it won't be limited only to post requests.This also means that
PUT,DELETE, andGETrequests will also use the raw parser, but I think that would be understood when a user creates a function and explicitly sets theRAW_BODYvariable totrueexpecting that the raw request body will be used.Which issue(s) this PR fixes
Fixes #191
How Has This Been Tested?
Created sample function to print request body for verifying the type received by the function code.
Using Postman, called the function with
application/jsonrequest.{ some: 'thing' }RAW_BODY: true-<Buffer 7b 0a 09 22 73 6f 6d 65 22 3a 20 22 74 68 69 6e 67 22 0a 7d>For the
MAX_JSON_SIZEtesting:I've used an example request from HugeJsonViewer example that is 1.5mb
Using Postman, called the function with a large request body:
Adding the following to the
fn1.yml:The result was successful:
Types of changes
Impact to existing users
None as the changes are wrapped in an environment variable. Verified the function works as expected without the env vars defined or used.
Checklist:
git commit -s