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(plugins): do not include user-input in UnsupportedMediaTypeError #1733

Merged
merged 1 commit into from Jan 2, 2019

Conversation

hekike
Copy link
Member

@hekike hekike commented Jan 2, 2019

User input in VError based errors (restify-errors) can cause the process to crash as VError uses the sprintf library internally which expects additional arguments for formatting symbols liek %s.

For example, a content-type %s would crash restify without this change.

@hekike
Copy link
Member Author

hekike commented Jan 2, 2019

LTS fails but @misterdjules is working on a fix, unrelated to this pr.

@misterdjules
Copy link
Contributor

Nitpicking and thinking out loud: should that be a minor version bump since we're adding an info field to the error object (which one could argue is technically an addition to the plug-in's API)?

Copy link
Contributor

@misterdjules misterdjules left a comment

Choose a reason for hiding this comment

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

Nitpicking again: the commit message seems to be slightly misleading: IIUC this change still includes user-input in the error, but instead of including it in the error message it includes it in a separate field. We might want to change the commit message to reflect that.

@hekike hekike force-pushed the fix/plugin-body-reader-error-message branch from 33b60e1 to e97e364 Compare January 2, 2019 21:20
@hekike hekike force-pushed the fix/plugin-body-reader-error-message branch from e97e364 to 691f455 Compare January 2, 2019 21:21
@hekike
Copy link
Member Author

hekike commented Jan 2, 2019

Minor sounds good to me, I think the reasoning is valid.
I changed the commit message two reflect your points, thanks!

@hekike hekike merged commit 06c220d into master Jan 2, 2019
@hekike hekike deleted the fix/plugin-body-reader-error-message branch January 2, 2019 23:42
@hekike
Copy link
Member Author

hekike commented Jan 2, 2019

released as restify@7.4.0

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

Successfully merging this pull request may close these issues.

None yet

3 participants