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(curl) remove boundary form content-type header #227

Merged
merged 1 commit into from Sep 3, 2021

Conversation

nateslo
Copy link
Member

@nateslo nateslo commented Sep 3, 2021

Resolves #226

This PR removes the boundary from the content-type header for curl multipart request. Curl appends its own boundary when using --form, resulting in a double boundary.

@@ -39,6 +40,22 @@ module.exports = function (source, options) {
code.push(opts.short ? '-0' : '--http1.0')
}

// if multipart form data, we want to remove the boundary
Copy link
Contributor

Choose a reason for hiding this comment

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

Per #226, we discussed just scoping this to fixing cURL due to how cURL handles --form vs raw data.

Did you attempt any kind of audit of other targets to see if they are impacted by similar issues?

Copy link
Member Author

@nateslo nateslo Sep 3, 2021

Choose a reason for hiding this comment

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

I have not investigate any other targets yet, this fix only pertains to curl multipart

if (contentTypeHeaderName && contentTypeHeader) {
// remove the leading semi colon and boundary
// up to the next semi colon or the end of string
const noBoundary = contentTypeHeader.replace(/; boundary.+?(?=(;|$))/, '')
Copy link
Contributor

Choose a reason for hiding this comment

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

I double checked these regex and functions as expected.

Screen Shot 2021-09-03 at 11 16 37 AM

Copy link
Contributor

@erunion erunion left a comment

Choose a reason for hiding this comment

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

big oof. thanks for fixing this.

if other targets need this boundary removed it might be worth refactoring this then into a part of the target config.

@nateslo
Copy link
Member Author

nateslo commented Sep 3, 2021

big oof. thanks for fixing this.

if other targets need this boundary removed it might be worth refactoring this then into a part of the target config.

No worries! Thanks for the review

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.

curl multipart/form-data snippet includes unwanted boundary
3 participants