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.

Sign up for GitHub

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

Jump to bottom

Boundaries for multipart requests in plain HTTP are incorrect #616

Open
thinkgruen opened this issue May 3, 2022 · 4 comments · May be fixed by #617
Open

Boundaries for multipart requests in plain HTTP are incorrect #616

thinkgruen opened this issue May 3, 2022 · 4 comments · May be fixed by #617
Labels
codegen good first issue Good for newcomers

Comments

@thinkgruen
Copy link

Describe the bug
Currently, the Code Generator for (plain) HTTP is using the same boundary everywhere instead of following the RFC. When trying to send the generated code through an HTTP client, e.g. in IntelliJ, the IDE will point out that the boundaries are incorrect, and the request will fail.

To Reproduce
Steps to reproduce the behavior:

  • create a POST using a form-data body
  • add a key "foo" and text value "bar"
  • generate the HTTP code

Expected code snippet and corresponding request
The generated code is correct according to the definition of the aforementioned RFC which means:

  • there is a boundary defined alongside Content-Type: multipart/form-data (let's say xyz)
  • the boundary is used to separate different form data with two additional hyphens before (--xyz)
  • the request ends with a boundary with two leading and trailing hyphens (--xyz--)

Screenshots
All boundaries are identical but should follow the rules above.
A screenshot of the generated code

Additional context
Version of postman-code-generators/Postman app: 9.16.1
@thinkgruen thinkgruen linked a pull request May 3, 2022 that will close this issue
Open
@VShingala VShingala added the good first issue Good for newcomers label Jun 7, 2024
@saksham-postman
Copy link

This issue was fixxed by this PR

Screenshot 2024-09-06 at 12 54 01 AM

@VShingala , this can be closed.

@theonly1me theonly1me removed their assignment Sep 5, 2024
@thinkgruen
Copy link
Author

imho I don't see how the merged fix is better than my proposition which also got submitted earlier.

It just adds additional characters that aren't required. comparing the Content-Length to my PR means that the examples went from 586 to 602 chars (587 or 588 in my proposal) and 581 to 597 (583 or 584 in my proposal).

additionally, the merged PR needed 14 changes as compared to my 6 - adding unneeded complexity (e.g. checking when a loop a is terminated at every iteration...)

@VShingala please consider reopening this as an optimization to the current solution and let me merge #617 as well.

@VShingala
Copy link
Member

@saksham-postman let's take detailed look here.

@VShingala VShingala reopened this Sep 6, 2024
@saksham-postman
Copy link

Followed up with #768 , merged develop into @thinkgruen branch to resolve conflicts. imo the changes looks good to me

@aman-v-singh aman-v-singh mentioned this issue Oct 7, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
codegen good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix incorrect multipart boundaries for plain HTTP thinkgruen/postman-code-generators
5 participants
@akshaydeo @VShingala @theonly1me @thinkgruen @saksham-postman