Skip to content

Fix Copy as CURL with Multipart #2740

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

Merged

Conversation

jgiovaresco
Copy link
Contributor

@jgiovaresco jgiovaresco commented Oct 19, 2020

The issue was located in the exportHar function. It was not handled file param correctly.
For this type of file, the value attribute is empty. Therefore we should take instead of the fileName

The issue also impacted the Generate code feature.

Closes #2282

Before

curl --request POST \
  --url http://localhost:3000/multipart \
  --header 'Content-Type: multipart/form-data' \
  --header 'content-type: multipart/form-data; boundary=---011000010111000001101001' \
  --form my_file=\
  --form my_field=my_value

After

curl --request POST \
  --url http://localhost:3000/multipart \
  --header 'Content-Type: multipart/form-data' \
  --header 'content-type: multipart/form-data; boundary=---011000010111000001101001' \
  --form my_second_file=@/Users/jgiovaresco/Documents/AFTER.gif \
  --form my_first_file=@/Users/jgiovaresco/Documents/Untitled \
  --form 'a_simple_value=my value'

@nijikokun
Copy link
Contributor

Awesome change, I assume this works for multi-file as well?

@jgiovaresco jgiovaresco force-pushed the fix/2281-copy-as-curl-with-multipart branch from 9791a6f to e2497b6 Compare October 20, 2020 06:10
@jgiovaresco
Copy link
Contributor Author

jgiovaresco commented Oct 20, 2020

Awesome change, I assume this works for multi-file as well?

You are assuming right ;)
I've updated the unit test to be sure. 😅

curl --request POST \
  --url http://localhost:3000/multipart \
  --header 'Content-Type: multipart/form-data' \
  --header 'content-type: multipart/form-data; boundary=---011000010111000001101001' \
  --form my_file=@/Users/jgiovaresco/Documents/Untitled \
  --form my_field=my_value \
  --form my_second_file=@/Users/jgiovaresco/Documents/AFTER.gif

@jgiovaresco jgiovaresco force-pushed the fix/2281-copy-as-curl-with-multipart branch 4 times, most recently from ca1d812 to 590211a Compare October 21, 2020 18:16
Copy link
Contributor

@reynolek reynolek left a comment

Choose a reason for hiding this comment

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

This change makes sense to me.

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

LGTM, sorry for the delay in getting to this!

@develohpanda develohpanda requested a review from DMarby November 19, 2020 00:44
@jgiovaresco jgiovaresco force-pushed the fix/2281-copy-as-curl-with-multipart branch from 51710e1 to 03a4880 Compare November 21, 2020 08:38
The issue was located in the `exportHar` function. It was not handled `file` param correctly.
For this type of file, the `value` attribute is empty, therefore we should take instead the `fileName`

Closes Kong#2282
@jgiovaresco jgiovaresco force-pushed the fix/2281-copy-as-curl-with-multipart branch from 03a4880 to ef24f72 Compare November 21, 2020 08:40
@jgiovaresco jgiovaresco requested a review from DMarby November 21, 2020 08:40
Copy link
Contributor

@DMarby DMarby left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing this @jgiovaresco!

@develohpanda develohpanda merged commit 6d259a4 into Kong:develop Nov 24, 2020
@jgiovaresco jgiovaresco deleted the fix/2281-copy-as-curl-with-multipart branch December 2, 2020 16:44
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.

Copy as CURL with Multipart
5 participants