From d32d8823e5b8d2deeaaa9cf94328122afc9fb492 Mon Sep 17 00:00:00 2001 From: Robert Ulejczyk Date: Mon, 26 Nov 2018 19:58:37 +0100 Subject: [PATCH] fixes for ruby 1.8 (#359) * add backports for URI in ruby 1.8 Form decode & encode methods have been added in 1.9 and some tests use these methods. This is a monkeypatch, but it's used only in test environment for ruby 1.8. It should be fine. The code was for the most part copied from the internet with minor tweaks. Given it's pretty much completely isolated from anything (will be used ONLY in ruby 1.8) I'm ok to leave it as is, with little care of accuracy and style. * fix `perform': select(): Operation now in progress` in ruby 1.8.7 This appears to be a result of misplaced `#if` and `#endif` caused the default fallback to never be executed and `rc` would remain with initialized value (`-1`) and raise an exception: ``` switch(rc) { case -1: if(errno != EINTR) { rb_raise(rb_eRuntimeError, "select(): %s", strerror(errno)); break; } ``` This was the original flow with indentation: ``` #if defined(HAVE_RB_THREAD_BLOCKING_REGION) || defined(HAVE_RB_THREAD_CALL_WITHOUT_GVL) fdset_args.maxfd = maxfd+1; fdset_args.fdread = &fdread; fdset_args.fdwrite = &fdwrite; fdset_args.fdexcep = &fdexcep; fdset_args.tv = &tv; #ifdef HAVE_RB_THREAD_CALL_WITHOUT_GVL rc = (int)(VALUE) rb_thread_call_without_gvl((void *(*)(void *))curb_select, &fdset_args, RUBY_UBF_IO, 0); #elif HAVE_RB_THREAD_BLOCKING_REGION rc = rb_thread_blocking_region(curb_select, &fdset_args, RUBY_UBF_IO, 0); #elif HAVE_RB_THREAD_FD_SELECT rc = rb_thread_fd_select(maxfd+1, &fdread, &fdwrite, &fdexcep, &tv); #else rc = rb_thread_select(maxfd+1, &fdread, &fdwrite, &fdexcep, &tv); #endif #endif ``` Ruby 1.8 does not define `HAVE_RB_THREAD_BLOCKING_REGION` nor `HAVE_RB_THREAD_CALL_WITHOUT_GVL` and no code is executed because the `else` clausule applies to inner `ifdef`. New flow, after my changes with indentation: ``` #if defined(HAVE_RB_THREAD_BLOCKING_REGION) || defined(HAVE_RB_THREAD_CALL_WITHOUT_GVL) fdset_args.maxfd = maxfd+1; fdset_args.fdread = &fdread; fdset_args.fdwrite = &fdwrite; fdset_args.fdexcep = &fdexcep; fdset_args.tv = &tv; #endif #ifdef HAVE_RB_THREAD_CALL_WITHOUT_GVL rc = (int)(VALUE) rb_thread_call_without_gvl((void *(*)(void *))curb_select, &fdset_args, RUBY_UBF_IO, 0); #elif HAVE_RB_THREAD_BLOCKING_REGION rc = rb_thread_blocking_region(curb_select, &fdset_args, RUBY_UBF_IO, 0); #elif HAVE_RB_THREAD_FD_SELECT rc = rb_thread_fd_select(maxfd+1, &fdread, &fdwrite, &fdexcep, &tv); #else rc = rb_thread_select(maxfd+1, &fdread, &fdwrite, &fdexcep, &tv); #endif ``` Here even when all the features are unavailable we have a fallback to `rc = rb_thread_select(maxfd+1, &fdread, &fdwrite, &fdexcep, &tv);`. This commit also changes `rb_thread_wait_for` to pass timeval explicityly rather than rely on `rb_time_timeval`. There must have been a change in ruby 1.9 because the `rb_time_timeval` does not work in 1.8.7. ``` - rb_thread_wait_for(rb_time_timeval(DBL2NUM(0.1))); + rb_thread_wait_for(tv_100ms); ``` * enable testing in ruby 1.8 This will allow us to continue support for 1.8 until we decide not to. There is no good reason for the code to not work in 1.8 and it will be good to know when we actually break compatibility. --- .travis.yml | 4 +++ Gemfile.ruby-1.8 | 6 ++++ Gemfile.ruby-1.8.lock | 21 ++++++++++++ Rakefile | 15 +++++---- ext/curb_multi.c | 12 ++++--- tests/helper.rb | 78 +++++++++++++++++++++++++++++++++++++++++++ tests/tc_curl_easy.rb | 2 +- 7 files changed, 125 insertions(+), 13 deletions(-) create mode 100644 Gemfile.ruby-1.8 create mode 100644 Gemfile.ruby-1.8.lock diff --git a/.travis.yml b/.travis.yml index 03133fd2..73105796 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,6 +10,10 @@ matrix: allow_failures: - rvm: rbx-3 fast_finish: true + include: + - rvm: 1.8.7 + dist: precise + gemfile: Gemfile.ruby-1.8 cache: bundler before_install: - gem update bundler diff --git a/Gemfile.ruby-1.8 b/Gemfile.ruby-1.8 new file mode 100644 index 00000000..9444dac8 --- /dev/null +++ b/Gemfile.ruby-1.8 @@ -0,0 +1,6 @@ +source 'https://rubygems.org' + +gemspec + +gem 'rake', '< 12' # as of 12.0 rake requires ruby 1.9 +gem 'test-unit', '< 3' diff --git a/Gemfile.ruby-1.8.lock b/Gemfile.ruby-1.8.lock new file mode 100644 index 00000000..16d6a72e --- /dev/null +++ b/Gemfile.ruby-1.8.lock @@ -0,0 +1,21 @@ +PATH + remote: . + specs: + curb (0.9.6) + +GEM + remote: https://rubygems.org/ + specs: + rake (10.5.0) + test-unit (2.5.5) + +PLATFORMS + ruby + +DEPENDENCIES + curb! + rake (< 12) + test-unit (< 3) + +BUNDLED WITH + 1.15.4 diff --git a/Rakefile b/Rakefile index d6045965..f2c219ce 100644 --- a/Rakefile +++ b/Rakefile @@ -2,11 +2,6 @@ # require 'rake/clean' require 'rake/testtask' -begin - require 'rdoc/task' -rescue LoadError => e - require 'rake/rdoctask' -end CLEAN.include '**/*.o' CLEAN.include "**/*.#{(defined?(RbConfig) ? RbConfig : Config)::MAKEFILE_CONFIG['DLEXT']}" @@ -89,12 +84,12 @@ if ENV['RELTEST'] else task :alltests => [:unittests, :bugtests] end - + Rake::TestTask.new(:unittests) do |t| t.test_files = FileList['tests/tc_*.rb'] t.verbose = false end - + Rake::TestTask.new(:bugtests) do |t| t.test_files = FileList['tests/bug_*.rb'] t.verbose = false @@ -136,6 +131,12 @@ end desc "Publish the RDoc documentation to project web site" task :doc_upload => [ :doc ] do + begin + require 'rdoc/task' + rescue LoadError => e + require 'rake/rdoctask' + end + if ENV['RELTEST'] announce "Release Task Testing, skipping doc upload" else diff --git a/ext/curb_multi.c b/ext/curb_multi.c index 043b39a8..a1384a65 100644 --- a/ext/curb_multi.c +++ b/ext/curb_multi.c @@ -537,6 +537,7 @@ VALUE ruby_curl_multi_perform(int argc, VALUE *argv, VALUE self) { #endif long timeout_milliseconds; struct timeval tv = {0, 0}; + struct timeval tv_100ms = {0, 100000}; VALUE block = Qnil; #if defined(HAVE_RB_THREAD_BLOCKING_REGION) || defined(HAVE_RB_THREAD_CALL_WITHOUT_GVL) struct _select_set fdset_args; @@ -551,7 +552,7 @@ VALUE ruby_curl_multi_perform(int argc, VALUE *argv, VALUE self) { rb_curl_multi_run( self, rbcm->handle, &(rbcm->running) ); rb_curl_multi_read_info( self, rbcm->handle ); if (block != Qnil) { rb_funcall(block, rb_intern("call"), 1, self); } - + do { while (rbcm->running) { @@ -593,7 +594,7 @@ VALUE ruby_curl_multi_perform(int argc, VALUE *argv, VALUE self) { if (maxfd == -1) { /* libcurl recommends sleeping for 100ms */ - rb_thread_wait_for(rb_time_timeval(DBL2NUM(0.1))); + rb_thread_wait_for(tv_100ms); rb_curl_multi_run( self, rbcm->handle, &(rbcm->running) ); rb_curl_multi_read_info( self, rbcm->handle ); if (block != Qnil) { rb_funcall(block, rb_intern("call"), 1, self); } @@ -606,12 +607,15 @@ VALUE ruby_curl_multi_perform(int argc, VALUE *argv, VALUE self) { create_crt_fd(&fdexcep, &crt_fdexcep); #endif -#if defined(HAVE_RB_THREAD_BLOCKING_REGION) || defined(HAVE_RB_THREAD_CALL_WITHOUT_GVL) + +#if (defined(HAVE_RB_THREAD_BLOCKING_REGION) || defined(HAVE_RB_THREAD_CALL_WITHOUT_GVL)) fdset_args.maxfd = maxfd+1; fdset_args.fdread = &fdread; fdset_args.fdwrite = &fdwrite; fdset_args.fdexcep = &fdexcep; fdset_args.tv = &tv; +#endif + #ifdef HAVE_RB_THREAD_CALL_WITHOUT_GVL rc = (int)(VALUE) rb_thread_call_without_gvl((void *(*)(void *))curb_select, &fdset_args, RUBY_UBF_IO, 0); #elif HAVE_RB_THREAD_BLOCKING_REGION @@ -622,8 +626,6 @@ VALUE ruby_curl_multi_perform(int argc, VALUE *argv, VALUE self) { rc = rb_thread_select(maxfd+1, &fdread, &fdwrite, &fdexcep, &tv); #endif -#endif - #ifdef _WIN32 cleanup_crt_fd(&fdread, &crt_fdread); cleanup_crt_fd(&fdwrite, &crt_fdwrite); diff --git a/tests/helper.rb b/tests/helper.rb index 87ac4070..c38e814a 100644 --- a/tests/helper.rb +++ b/tests/helper.rb @@ -207,3 +207,81 @@ def server_setup(port=9129,servlet=TestServlet) rescue Errno::EADDRINUSE end end + + + +# Backport for Ruby 1.8 +module Backports + module Ruby18 + module URIFormEncoding + TBLENCWWWCOMP_ = {} + TBLDECWWWCOMP_ = {} + + def encode_www_form_component(str) + if TBLENCWWWCOMP_.empty? + 256.times do |i| + TBLENCWWWCOMP_[i.chr] = '%%%02X' % i + end + TBLENCWWWCOMP_[' '] = '+' + TBLENCWWWCOMP_.freeze + end + str.to_s.gsub( /([^*\-.0-9A-Z_a-z])/ ) {|*| TBLENCWWWCOMP_[$1] } + end + + def decode_www_form_component(str) + if TBLDECWWWCOMP_.empty? + 256.times do |i| + h, l = i>>4, i&15 + TBLDECWWWCOMP_['%%%X%X' % [h, l]] = i.chr + TBLDECWWWCOMP_['%%%x%X' % [h, l]] = i.chr + TBLDECWWWCOMP_['%%%X%x' % [h, l]] = i.chr + TBLDECWWWCOMP_['%%%x%x' % [h, l]] = i.chr + end + TBLDECWWWCOMP_['+'] = ' ' + TBLDECWWWCOMP_.freeze + end + + raise ArgumentError, "invalid %-encoding (#{str.dump})" unless /\A(?:%[[:xdigit:]]{2}|[^%]+)*\z/ =~ str + str.gsub( /(\+|%[[:xdigit:]]{2})/ ) {|*| TBLDECWWWCOMP_[$1] } + end + + def encode_www_form( enum ) + enum.map do |k,v| + if v.nil? + encode_www_form_component(k) + elsif v.respond_to?(:to_ary) + v.to_ary.map do |w| + str = encode_www_form_component(k) + unless w.nil? + str << '=' + str << encode_www_form_component(w) + end + end.join('&') + else + str = encode_www_form_component(k) + str << '=' + str << encode_www_form_component(v) + end + end.join('&') + end + + WFKV_ = '(?:%\h\h|[^%#=;&])' + def decode_www_form(str, _) + return [] if str.to_s == '' + + unless /\A#{WFKV_}=#{WFKV_}(?:[;&]#{WFKV_}=#{WFKV_})*\z/ =~ str + raise ArgumentError, "invalid data of application/x-www-form-urlencoded (#{str})" + end + ary = [] + $&.scan(/([^=;&]+)=([^;&]*)/) do + ary << [decode_www_form_component($1, enc), decode_www_form_component($2, enc)] + end + ary + end + end + end +end + +unless URI.methods.include?(:encode_www_form) + URI.extend(Backports::Ruby18::URIFormEncoding) +end diff --git a/tests/tc_curl_easy.rb b/tests/tc_curl_easy.rb index 3f0352ec..4bb1c135 100644 --- a/tests/tc_curl_easy.rb +++ b/tests/tc_curl_easy.rb @@ -642,7 +642,7 @@ def test_cookiejar def test_cookielist c = Curl::Easy.new TestServlet.url c.enable_cookies = true - c.post_body = URI.encode_www_form c: 'somename=somevalue' + c.post_body = URI.encode_www_form('c' => 'somename=somevalue') assert_nil c.cookielist c.perform assert_match(/somevalue/, c.cookielist.join(''))