-
Notifications
You must be signed in to change notification settings - Fork 118
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
Add support for CSP Reporting & ndJSON #397
Conversation
CSP Reporting (`application/csp-report`) posts use standard JSON syntax, while ndJSON (`application/x-ndjson`) requires to be reformatted as an array of structs.
Interesting. 🤔 I assume the CSP reports part is for automted CSP violation reporting? Adding NDJson support is an interesting idea. I don't have any problems with it, especially since it's just a couple of lines. I assume you have a use-case in mind? The code additions look good to me. We'll need some documentation to go with them. Would you mind also submitting a PR to @atuttle/TaffyDocs so that I can merge both at the same time? It should be added to the 3.3.0.md file. since these are not new methods or configuration, and for lack of an existing designated location for them, please add a section immediately under Override the request method with a header with the heading "Special case request content types" with a brief description of each, and any relevant canonical links that would be helpful. |
I recently worked with Campaigner.com. Their webhook options were either 1) a multiple form post (with duplicate field names) or 2) ndJSON. Since ndJSON isn't valid JSON, the Taffy core was ignoring it unless the string is wrapped in an array. All of my personal tests were successful, but their webhook was still broken. I finally discovered that their webhook wasn't sending NOTE: Add this before the code you just added if you wish. You could even modify it in the case an invalid <cfif not len(requestObj.contentType) and isSimpleValue(requestObj.body)
and len(requestObj.body) and isJSON("[" & javacast("string", requestObj.body).replace("\n", ",") & "]")>
<cfset requestObj.contentType = "application/x-ndjson">
</cfif> |
Sounds good to me. This should make a fine addition.
I don't fully follow what you're suggesting here. FYI if you make the change to your |
Update to [ndJSON](http://ndjson.org/) string detection as some service providers don't follow [RFC4288](https://www.ietf.org/rfc/rfc4288.txt) and fail to pass `application/x-ndjson` as the `content-type` header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updates to ndJSON detection as some service provider don't pass correct content-type
headers. Also update to detect browser CSP submissions as json.
That makes sense, thanks. Code updates look good. Just need some sort of documentation about these improvements so they don't become forgotten/undocumented features. |
A major data provider uses ndJSON, but doesn't send `content-type` headers. Add support to correctly set type to `application/json` if body is JSON without any new lines.
I've updated the documentation. I also updated the detection based on my experience with a third-party provider that isn't passing any |
<cfif isSimpleValue(requestObj.body) and listlen(requestObj.Body,chr(10)) GT 1 and not isJSON(requestObj.body) and isJSON("[" & javacast("string", requestObj.body).replace("\n", ",") & "]")> | ||
<cfif not len(requestObj.contentType) and isSimpleValue(requestObj.body) and isJSON(requestObj.body)> | ||
<cfset requestObj.contentType = "application/json"> | ||
<cfelseif isSimpleValue(requestObj.body) and listlen(requestObj.Body,chr(10)) GT 1 and not isJSON(requestObj.body) and isJSON("[" & javacast("string", requestObj.body).replace("\n", ",") & "]")> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just had a thought: what if there's only 1 row in the ndJson body? your listLen()
param will evaluate to false. 🤔
Also:
Where? I don't see a PR in the TaffyDocs repo for it, nor do I see it in any of the comments in this PR. Granted it's getting late here and my eyes are tired, so maybe I'm just missing it. |
Resolves #383
CSP Reporting (
application/csp-report
) posts use standard JSON syntax, while ndJSON (application/x-ndjson
) requires to be reformatted as an array of structs.