Skip to content

fix: improve binary data detection in HTTP transport #468

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

Merged
merged 2 commits into from
Feb 2, 2022

Conversation

lance
Copy link
Member

@lance lance commented Feb 1, 2022

Previously, we were only checking for a single typed array, UInt32Array but that's not quite broad enough, since binary data can exist in various forms. This means of checking for binary data, using types.isTypedArray is not foolproof, but is better than previous, as the previous implementation would not recognize a PNG file read from disk via fs.readFileSync as binary data. Now it will.

Signed-off-by: Lance Ball [email protected]

Previously, we were only checking for a single typed array,
`UInt32Array` but that's not quite broad enough, since binary
data can exist in various forms. This means of checking for
binary data, using `types.isTypedArray` is not foolproof, but
is better than previous, as the previous implementation would
not recognize a PNG file read from disk via `fs.readFileSync`
as binary data. Now it will.

Signed-off-by: Lance Ball <[email protected]>
@lance lance added the type/fix A change that fixes something that is broken label Feb 1, 2022
@lance lance requested a review from a team February 1, 2022 15:30
@lance lance self-assigned this Feb 1, 2022
Signed-off-by: Lance Ball <[email protected]>
lance added a commit to lance/faas-js-runtime that referenced this pull request Feb 1, 2022
@lance lance merged commit cd4dea9 into cloudevents:main Feb 2, 2022
lance added a commit to nodeshift/faas-js-runtime that referenced this pull request Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/fix A change that fixes something that is broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants