taf2 / curb

Ruby bindings for libcurl
Other
1.29k stars 227 forks source link

error running the curb test suite with curl 8.7.1 #451

Open lnussbaum opened 1 month ago

lnussbaum commented 1 month ago

this is from the Debian package build, but it doesn't look specific to Debian:

RUBYLIB=. GEM_PATH=/home/vagrant/ruby-curb-1.0.5/debian/ruby-curb/usr/lib/x86_64-linux-gnu/rubygems-integration/3.1.0:/home/vagrant/ruby-curb-1.0.5/debian/.debhelper/generated/_source/home/.local/share/gem/ruby/3.1.0:/var/lib/gems/3.1.0:/usr/local/lib/ruby/gems/3.1.0:/usr/lib/ruby/gems/3.1.0:/usr/lib/x86_64-linux-gnu/ruby/gems/3.1.0:/usr/share/rubygems-integration/3.1.0:/usr/share/rubygems-integration/all:/usr/lib/x86_64-linux-gnu/rubygems-integration/3.1.0 ruby3.1 -S rake --rakelibdir /gem2deb-nonexistent -f debian/ruby-tests.rake
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl_multi.rb:189: warning: assigned but unused variable - in_file_stack
Loaded suite /usr/share/rubygems-integration/all/gems/rake-13.2.1/lib/rake/rake_test_loader
Started
.............................E
========================================================================================================================================
Error: test_easy_http_verbs(TestCurbCurlEasy): Curl::Err::ReadError: Failed to open/read local data from file/application: client read function EOF fail, only only 4/5 of needed bytes read
/home/vagrant/ruby-curb-1.0.5/lib/curl/easy.rb:80:in `perform'
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl_easy.rb:1064:in `http_put'
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl_easy.rb:1064:in `test_easy_http_verbs'
     1061:     assert_equal "POST\n", curl.body_str
     1062:     curl.http('PURGE')
     1063:     assert_equal 'PURGE', curl.body_str
  => 1064:     curl.http_put('hello')
     1065:     assert_equal "PUT\nhello", curl.body_str
     1066:     curl.http('COPY')
     1067:     assert_equal 'COPY', curl.body_str
========================================================================================================================================
.................................................................E
========================================================================================================================================
Error: test_put_data(TestCurbCurlEasy): Curl::Err::ReadError: Failed to open/read local data from file/application: client read function EOF fail, only only 6/7 of needed bytes read
/home/vagrant/ruby-curb-1.0.5/lib/curl/easy.rb:80:in `perform'
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl_easy.rb:885:in `test_put_data'
     882:     curl = Curl::Easy.new(TestServlet.url)
     883:     curl.put_data = 'message'
     884:     
  => 885:     curl.perform
     886:     
     887:     assert_match(/^PUT/, curl.body_str)
     888:     assert_match(/message$/, curl.body_str)
========================================================================================================================================
E
========================================================================================================================================
Error: test_put_data_null_bytes(TestCurbCurlEasy): Curl::Err::ReadError: Failed to open/read local data from file/application: client read function EOF fail, only only 2/3 of needed bytes read
/home/vagrant/ruby-curb-1.0.5/lib/curl/easy.rb:80:in `perform'
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl_easy.rb:896:in `test_put_data_null_bytes'
     893:     curl = Curl::Easy.new(TestServlet.url)
     894:     curl.put_data = "a\0b"
     895:  
  => 896:     curl.perform
     897:     
     898:     assert_match(/^PUT/, curl.body_str)
     899:     assert_match("a\0b", curl.body_str)
========================================================================================================================================
.E
========================================================================================================================================
Error: test_put_remote(TestCurbCurlEasy): Curl::Err::ReadError: Failed to open/read local data from file/application: client read function EOF fail, only only 6/7 of needed bytes read
/home/vagrant/ruby-curb-1.0.5/lib/curl/easy.rb:80:in `perform'
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl_easy.rb:873:in `http_put'
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl_easy.rb:873:in `test_put_remote'
     870:   def test_put_remote
     871:     curl = Curl::Easy.new(TestServlet.url)
     872:     curl.headers['Content-Type'] = 'application/json'
  => 873:     assert curl.http_put("message")
     874:     assert_match(/^PUT/, curl.body_str)
     875:     assert_match(/message$/, curl.body_str)
     876:     assert_match(/message$/, curl.body)
========================================================================================================================================
...............................O
========================================================================================================================================
Omission: Skip, libcurl too old (< 7.22.0) [test_connection_keepalive(TestCurbCurlMulti)]
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl_multi.rb:16:in `test_connection_keepalive'
========================================================================================================================================
......E
========================================================================================================================================
Error: test_multi_easy_http_01(TestCurbCurlMulti):
  Curl::Err::AbortedByCallbackError: <200> expected but was
  <0>.
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl_multi.rb:523:in `block in test_multi_easy_http_01'
     520:       { :url => TestServlet.url, :method => :get }
     521:     ]
     522:     Curl::Multi.http(urls, {:pipeline => true}) do|easy, code, method|
  => 523:       assert_equal 200, code
     524:       case method
     525:       when :post
     526:         assert_match(/POST/, easy.body_str)
/home/vagrant/ruby-curb-1.0.5/lib/curl/multi.rb:143:in `block (2 levels) in http'
/home/vagrant/ruby-curb-1.0.5/lib/curl/multi.rb:164:in `perform'
/home/vagrant/ruby-curb-1.0.5/lib/curl/multi.rb:164:in `http'
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl_multi.rb:522:in `test_multi_easy_http_01'
========================================================================================================================================
..E
========================================================================================================================================
Error: test_multi_easy_put_01(TestCurbCurlMulti):
  Curl::Err::AbortedByCallbackError: </PUT/> was expected to be =~
  <"">.
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl_multi.rb:508:in `block in test_multi_easy_put_01'
     505:            { :url => TestServlet.url, :method => :put, :put_data => "message",
     506:              :headers => {'Content-Type' => 'application/json' } }]
     507:     Curl::Multi.put(urls, {}, {:pipeline => true}) do|easy|
  => 508:       assert_match(/PUT/, easy.body_str)
     509:       assert_match(/message/, easy.body_str)
     510:     end
     511:   end
/home/vagrant/ruby-curb-1.0.5/lib/curl/multi.rb:67:in `block in put'
/home/vagrant/ruby-curb-1.0.5/lib/curl/multi.rb:143:in `block (2 levels) in http'
/home/vagrant/ruby-curb-1.0.5/lib/curl/multi.rb:164:in `perform'
/home/vagrant/ruby-curb-1.0.5/lib/curl/multi.rb:164:in `http'
/home/vagrant/ruby-curb-1.0.5/lib/curl/multi.rb:67:in `put'
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl_multi.rb:507:in `test_multi_easy_put_01'
========================================================================================================================================
..................................E
========================================================================================================================================
Error: test_put(TestCurl): Curl::Err::ReadError: Failed to open/read local data from file/application: client read function EOF fail, only only 6/7 of needed bytes read
/home/vagrant/ruby-curb-1.0.5/lib/curl/easy.rb:80:in `perform'
/home/vagrant/ruby-curb-1.0.5/lib/curl.rb:26:in `http'
/home/vagrant/ruby-curb-1.0.5/lib/curl.rb:26:in `http'
/home/vagrant/ruby-curb-1.0.5/lib/curl.rb:39:in `put'
/home/vagrant/ruby-curb-1.0.5/tests/tc_curl.rb:20:in `test_put'
     17:   end
     18: 
     19:   def test_put
  => 20:     curl = Curl.put(TestServlet.url, {:foo => "bar"})
     21:     assert_equal "PUT\nfoo=bar",  curl.body_str
     22:   end
     23: 
========================================================================================================================================
....
Finished in 13.048177976 seconds.
----------------------------------------------------------------------------------------------------------------------------------------
180 tests, 614 assertions, 0 failures, 7 errors, 0 pendings, 1 omissions, 0 notifications
96.0894% passed
----------------------------------------------------------------------------------------------------------------------------------------
13.80 tests/s, 47.06 assertions/s

The corresponding Debian bug is https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1069258

In the Debian bug, someone points to https://curl.se/mail/lib-2024-03/0054.html , which might indicate that this is a bug in curb, not in curl.

It works fine with curl 8.6.0.

lares24 commented 1 month ago

In previous versions of curl, the read callback was sent a buffer of 65KB. Curl 8.7.1 now appears to use a buffer size equal to the object being PUT.

This in turn exposes a possible bug in https://github.com/taf2/curb/blob/13144ec5d50ffea0460298cc5de8a0b33db78d22/ext/curb_easy.c#L51

The remaining < read_bytes section works with the previously large buffer whereas the section marked // they're equal for some reason decrements the read_bytes value, causing the byte mismatch error in curl.

It's unclear why this behavior exists; changing remaining < read_bytes to remaining <= read_bytes is likely a sufficient fix for this issue.

mtasaka commented 5 days ago

git bisect for curl.git shows the following change brings about this error: https://github.com/curl/curl/commit/9369c30cd87c041cf983bcdfabd1570980abbaf6

(as the above debian bug says)

mtasaka commented 5 days ago

I have not read APIs of curl, and have not read the above change in detail, however reading the comment of the above commit:

test547, test555, test1620: fix the length check in the lib code to
  only fail for reads *smaller* than expected. This was a bug in the
  test code that never triggered in the old implementation.

and the actual code change, such as https://github.com/curl/curl/commit/9369c30cd87c041cf983bcdfabd1570980abbaf6#diff-bfeebefe5ca654a47ef7409092bf938d0002810757743093dcabc5fb301ad9feR50

seems to be suggesting that curb side code should be changed as:

diff --git a/ext/curb_easy.c b/ext/curb_easy.c
index 9054be6..74fa0e6 100644
--- a/ext/curb_easy.c
+++ b/ext/curb_easy.c
@@ -79,7 +79,7 @@ static size_t read_data_handler(void *ptr,
     remaining = len - rbcu->offset;
     str_ptr = RSTRING_PTR(str);

-    if( remaining < read_bytes ) {
+    if( remaining <= read_bytes ) {
       if( remaining > 0 ) {
         memcpy(ptr, str_ptr+rbcu->offset, remaining);
         read_bytes = remaining;

, as already mentioned above https://github.com/taf2/curb/blob/13144ec5d50ffea0460298cc5de8a0b33db78d22/ext/curb_easy.c#L82 And actually the above curb side change works with curl 8.6.0, 8.7.0, 8.7.1 and 8.8.0 . Would you look at this?