-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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: curlify agnostic to order of header values #6152
fix: curlify agnostic to order of header values #6152
Conversation
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.
I've suggested multiple changes to this PR. Please have a look at them. I also thing this change introduces possible bug.
5ebe258
to
f35501e
Compare
f35501e
to
33705ff
Compare
@char0n Updated. Please take another look. Thanks! |
Refs swagger-api#6082 * use curlify flag isMultipartFormDataRequest * curlify test updated
33705ff
to
7a22752
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.
Approved with 3 suggestions. Nice job!
Co-authored-by: Vladimir Gorej <vladimir.gorej@gmail.com>
Co-authored-by: Vladimir Gorej <vladimir.gorej@gmail.com>
Co-authored-by: Vladimir Gorej <vladimir.gorej@gmail.com>
Refs swagger-api#6082 * use curlify flag isMultipartFormDataRequest * curlify test updated Co-authored-by: Vladimir Gorej <vladimir.gorej@gmail.com>
Description
In
curlify
, thetype
would overwrite the string value to the latest header value. In the case of usingrequestInterceptor
that appends a header, the final string would be whatever was provided by therequestInterceptor
.This fix will try to update
isMultipartFormDataRequest
until it istrue
.This change also reorganizes and fixes the linting in the
curlify
test fileMotivation and Context
Fixes #6082
How Has This Been Tested?
New mocha tests that places leading/trailing custom header. There already exists a mocha test with no custom header. Note, although the term
custom
is used, any header and order should not impact the curlify result.Screenshots (if appropriate):
Checklist
My PR contains...
src/
is unmodified: changes to documentation, CI, metadata, etc.)package.json
)My changes...
Documentation
Automated tests