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

Issue #797: Handle multi-byte body in Net::HTTP on ruby 2.6.0 #814

Merged
merged 1 commit into from
Feb 1, 2019

Conversation

waltjones
Copy link
Contributor

#797

Ruby 2.6.0 has a bug where large payloads for Net::HTTP::Post are silently truncated. It turns out that this is caused by incorrectly calculating the byte length for payloads with multi-byte characters, and therefore the bug is limited to requests that do have multi-byte characters. Also, the error involves the length returned by IO#io.write_nonblock.

Several solutions have been considered. Splitting payloads in advance cannot be guaranteed to work in all cases because the behavior of IO#io.write_nonblock is platform dependent. A threshold length that works most of the time can't be known to work in all cases. Monkey patching Net::BufferedIO#write0 would work, however the solution in this PR does not require patching core Ruby.

In this PR, only if the current Ruby is 2.6.0 and the current payload is found to contain multi-byte characters, we unpack the string and repack as single bytes characters. This avoids the bug in Ruby, and is handled correctly on the receiving end (Rollbar notifier API.)

@waltjones waltjones changed the title Handle multi-byte body in Net::HTTP on ruby 2.6.0 Issue #797: Handle multi-byte body in Net::HTTP on ruby 2.6.0 Feb 1, 2019
Copy link
Contributor

@rokob rokob left a comment

Choose a reason for hiding this comment

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

Tested it and it works for me, so ship it.

@waltjones waltjones merged commit 4a86624 into master Feb 1, 2019
@waltjones waltjones deleted the ruby260-multibyte branch March 8, 2019 18: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.

2 participants