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

Add Content-* headers for empty POST requests #249

Closed
wants to merge 2 commits into from
Closed

Add Content-* headers for empty POST requests #249

wants to merge 2 commits into from

Conversation

dd32
Copy link
Member

@dd32 dd32 commented Oct 21, 2016

Sends the Content-Length and Content-Type headers even for empty POST requests.

The test passes regardless, as it seems that the Heroku test app adds the Content-Length headers on incoming requests regardless of if it's sent. Testing against http://httpbin.org/post shows that the header isn't/is sent though.

Fixes #248

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

@dd32 Thank you for this PR and sorry for the long wait.

The change make sense to me.

Looks like #318 also addresses this. Would it be beneficial to add the unit tests from #318 into this PR ? or combine them in some way ?

Also, the assertEquals() should probably be changed to an assertSame() ?

Other than that, would you mind rebasing the PR so it becomes mergable ?
If you don't want to or don't have the time, please let me know.

@dd32
Copy link
Member Author

dd32 commented Sep 26, 2019

I'm unable to rebase this as I no longer have the original repository it was made in.

It looks like a new PR combining this & #318 would make sense (I don't have the bandwidth for this right now)

@jrfnl
Copy link
Member

jrfnl commented Sep 26, 2019

@dd32 Thanks for letting me know. I've created PR #368 now which consolidates both previous PRs. Feedback welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Content-Length header not specified for empty POST requests with fsockopen transport
2 participants