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(fixRequestBody): fix request body for empty JSON object requests #640

Merged
merged 2 commits into from
Jan 23, 2022
Merged

fix(fixRequestBody): fix request body for empty JSON object requests #640

merged 2 commits into from
Jan 23, 2022

Conversation

mhassan1
Copy link
Contributor

Description

This PR fixes the fixRequestBody utility so that it treats empty object bodies like non-empty object bodies and fixes them, as expected.

Motivation and Context

The fixRequestBody utility contains a check for empty objects that results in early return:

if (!requestBody || !Object.keys(requestBody).length) {
return;
}

Currently, JSON requests with empty object payloads do not get their request bodies fixed by this utility, and the request to the target hangs while the target waits for a payload that will never arrive.

This PR removes the special handling for empty objects and treats them like any other object.

Resolves #639.

How has this been tested?

I ran the reproduction steps in #639 before and after the code change. I also updated automated tests.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Copy link

@eemelipa eemelipa left a comment

Choose a reason for hiding this comment

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

LGTM

Faced the same issue. Allowing empty objects in the body fixes it

@HappyColour
Copy link

This PR has been released right?

@eemelipa
Copy link

This PR has been released right?

I don't think so. The PR is still open and the library doesn't have the fix

@HappyColour
Copy link

This PR has been released right?

I don't think so. The PR is still open and the library doesn't have the fix

Thx bro @eemelipa,please confirm this pr has been merged @mhassan1.

@mhassan1
Copy link
Contributor Author

No, it is still open.

@chimurai
Copy link
Owner

chimurai commented Nov 28, 2021

Hi @mhassan1. I left a remark in the PR a while ago which hasn't been resolved.

Happy to merge it after it's fixed.

Edit: Sorry, review was pending all the time. (bad github ui/ux) Should be visible now.

@chimurai chimurai self-requested a review November 28, 2021 11:45
@mhassan1
Copy link
Contributor Author

mhassan1 commented Dec 2, 2021

@chimurai This is ready for re-review.

@coveralls
Copy link

coveralls commented Dec 2, 2021

Coverage Status

Coverage remained the same at 98.86% when pulling ece40cb on mhassan1:fix/fix-request-body-empty-json into 6b5d7a8 on chimurai:master.

@chimurai chimurai changed the title Fix request body for empty JSON object requests fix(fixRequestBody): fix request body for empty JSON object requests Jan 23, 2022
@chimurai chimurai merged commit 2bddd38 into chimurai:master Jan 23, 2022
@chimurai
Copy link
Owner

Thanks @mhassan1 !

@chimurai
Copy link
Owner

published in http-proxy-middleware@2.0.2

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.

Empty JSON object does not get fixed by fixRequestBody
5 participants