Skip to content
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

fix: Do not pass numeric arguments to res.send #242

Merged
merged 3 commits into from
Jan 29, 2021

Conversation

sk-
Copy link
Contributor

@sk- sk- commented Dec 24, 2020

Fixes #239.

res.send when receiving a sole numeric argument will set thee statuc code to that number.

This breaks HTTP communication as some numbers are not valid HTTP response codes.

For example, in the original issue the code is being set to 2. Which probably comes from the UNKNOWN error response of an RPC call.

This makes that the function is reported as having a connection error.

Fixes GoogleCloudPlatform#239.

res.send when receiving a sole numeric argument will set thee statuc code to that number.

This breaks HTTP communication as some numbers are not valid HTTP response codes.

For example, in the original issue the code is being set to 2. Which probably comes from the UNKNOWN error response of an RPC call.

This makes that the function is reported as having a `connection error`.
@google-cla google-cla bot added the cla: yes label Dec 24, 2020
Copy link
Contributor

@grant grant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay in reviewing. To fix the error, I suggest the logic be a bit simpler – we convert the crash response to always be a string.

src/logger.ts Outdated Show resolved Hide resolved
@grant grant added the automerge Summon MOG for automerging label Jan 29, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit a8ace7b into GoogleCloudPlatform:master Jan 29, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Summon MOG for automerging label Jan 29, 2021
@grant
Copy link
Contributor

grant commented Jan 29, 2021

Will be published in next release of the FF. If there's urgency to publish this, ping here.

@Jkudjo
Copy link

Jkudjo commented Jul 2, 2022

found this useful in my application. Gracias

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

express deprecated res.send(status): Use res.sendStatus(status) instead
3 participants