-
Notifications
You must be signed in to change notification settings - Fork 26
Fix response content type header check when content type header contains charset substring #13
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 response content type header check when content type header contains charset substring #13
Conversation
Merge updates
85ca715
to
d585eee
Compare
…ins charset substring
d585eee
to
3cefaae
Compare
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.
one question, looks good otherwise.
callback(undefined, responseBody, decryptedData); | ||
} | ||
}) | ||
.catch(() => callback(`Failed to decrypt response for ${httpMethod} request`, res, res)); |
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.
why are we passing res parameter 2 times?
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.
For convension in callback we use 1st parameter for error, 2nd parameter for response body and 3rd parameter for entire response. As this part of code is inside processEncryptedResponse method res parameter is actually the response body, so in that case we could pass 3rd parameter as undefined, but I suppose that in that case we can pass response body(res) as the 3rd response parameter
No description provided.