fix faulty rubocop cop 2

load integrations by default when possible

Closes #192

See merge request os85/httpx!227

Gemfile

@@ -17,7 +17,7 @@ group :test do
   gem "minitest-proveit"
   gem "ruby-ntlm"
   gem "sentry-ruby" if RUBY_VERSION >= "2.4.0"
-  gem "spy", "< 1.0.4" #  TODO: remove this once upstream fixes bug
+  gem "spy"
   if RUBY_VERSION < "2.3.0"
     gem "webmock", "< 3.15.0"
   else

integration_tests/datadog_test.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
+require "ddtrace"
 require "test_helper"
 require "support/http_helpers"
-require "ddtrace"
 require "httpx/adapters/datadog"
 
 class DatadogTest < Minitest::Test

integration_tests/sentry_test.rb

@@ -1,118 +1,118 @@
 # frozen_string_literal: true
 
-require "logger"
-require "stringio"
-require "test_helper"
-require "support/http_helpers"
-begin
+if RUBY_VERSION >= "2.4.0"
+  require "logger"
+  require "stringio"
+  require "sentry-ruby"
+  require "test_helper"
+  require "support/http_helpers"
   require "httpx/adapters/sentry"
-rescue LoadError
+
+  class SentryTest < Minitest::Test
+    include HTTPHelpers
+
+    DUMMY_DSN = "http://12345:67890@sentry.localdomain/sentry/42"
+
+    def test_sentry_send_yes_pii
+      before_pii = Sentry.configuration.send_default_pii
+      begin
+        Sentry.configuration.send_default_pii = true
+
+        transaction = Sentry.start_transaction
+        Sentry.get_current_scope.set_span(transaction)
+
+        uri = build_uri("/get")
+
+        response = HTTPX.get(uri, params: { "foo" => "bar" })
+
+        verify_status(response, 200)
+        verify_spans(transaction, response, description: "GET #{uri}?foo=bar")
+      ensure
+        Sentry.configuration.send_default_pii = before_pii
+      end
+    end
+
+    def test_sentry_send_no_pii
+      before_pii = Sentry.configuration.send_default_pii
+      begin
+        Sentry.configuration.send_default_pii = false
+
+        transaction = Sentry.start_transaction
+        Sentry.get_current_scope.set_span(transaction)
+
+        uri = build_uri("/get")
+
+        response = HTTPX.get(uri, params: { "foo" => "bar" })
+
+        verify_status(response, 200)
+        verify_spans(transaction, response, description: "GET #{uri}")
+      ensure
+        Sentry.configuration.send_default_pii = before_pii
+      end
+    end
+
+    def test_sentry_post_request
+      before_pii = Sentry.configuration.send_default_pii
+      begin
+        Sentry.configuration.send_default_pii = true
+        transaction = Sentry.start_transaction
+        Sentry.get_current_scope.set_span(transaction)
+
+        response = HTTPX.post(build_uri("/post"), form: { foo: "bar" })
+        verify_status(response, 200)
+        verify_spans(transaction, response, verb: "POST")
+      ensure
+        Sentry.configuration.send_default_pii = before_pii
+      end
+    end
+
+    def test_sentry_multiple_requests
+      transaction = Sentry.start_transaction
+      Sentry.get_current_scope.set_span(transaction)
+
+      responses = HTTPX.get(build_uri("/status/200"), build_uri("/status/404"))
+      verify_status(responses[0], 200)
+      verify_status(responses[1], 404)
+      verify_spans(transaction, *responses)
+    end
+
+    private
+
+    def verify_spans(transaction, *responses, verb: nil, description: nil)
+      assert transaction.span_recorder.spans.count == responses.size + 1
+      assert transaction.span_recorder.spans[0] == transaction
+
+      response_spans = transaction.span_recorder.spans[1..-1]
+
+      responses.each_with_index do |response, idx|
+        request_span = response_spans[idx]
+        assert request_span.op == "httpx.client"
+        assert !request_span.start_timestamp.nil?
+        assert !request_span.timestamp.nil?
+        assert request_span.start_timestamp != request_span.timestamp
+        assert request_span.description == (description || "#{verb || "GET"} #{response.uri}")
+        assert request_span.data == { status: response.status }
+      end
+    end
+
+    def setup
+      super
+
+      mock_io = StringIO.new
+      mock_logger = Logger.new(mock_io)
+
+      Sentry.init do |config|
+        config.traces_sample_rate = 1.0
+        config.logger = mock_logger
+        config.dsn = DUMMY_DSN
+        config.transport.transport_class = Sentry::DummyTransport
+        # so the events will be sent synchronously for testing
+        config.background_worker_threads = 0
+      end
+    end
+
+    def origin
+      "https://#{httpbin}"
+    end
+  end
 end
-
-class SentryTest < Minitest::Test
-  include HTTPHelpers
-
-  DUMMY_DSN = "http://12345:67890@sentry.localdomain/sentry/42"
-
-  def test_sentry_send_yes_pii
-    before_pii = Sentry.configuration.send_default_pii
-    begin
-      Sentry.configuration.send_default_pii = true
-
-      transaction = Sentry.start_transaction
-      Sentry.get_current_scope.set_span(transaction)
-
-      uri = build_uri("/get")
-
-      response = HTTPX.get(uri, params: { "foo" => "bar" })
-
-      verify_status(response, 200)
-      verify_spans(transaction, response, description: "GET #{uri}?foo=bar")
-    ensure
-      Sentry.configuration.send_default_pii = before_pii
-    end
-  end
-
-  def test_sentry_send_no_pii
-    before_pii = Sentry.configuration.send_default_pii
-    begin
-      Sentry.configuration.send_default_pii = false
-
-      transaction = Sentry.start_transaction
-      Sentry.get_current_scope.set_span(transaction)
-
-      uri = build_uri("/get")
-
-      response = HTTPX.get(uri, params: { "foo" => "bar" })
-
-      verify_status(response, 200)
-      verify_spans(transaction, response, description: "GET #{uri}")
-    ensure
-      Sentry.configuration.send_default_pii = before_pii
-    end
-  end
-
-  def test_sentry_post_request
-    before_pii = Sentry.configuration.send_default_pii
-    begin
-      Sentry.configuration.send_default_pii = true
-      transaction = Sentry.start_transaction
-      Sentry.get_current_scope.set_span(transaction)
-
-      response = HTTPX.post(build_uri("/post"), form: { foo: "bar" })
-      verify_status(response, 200)
-      verify_spans(transaction, response, verb: "POST")
-    ensure
-      Sentry.configuration.send_default_pii = before_pii
-    end
-  end
-
-  def test_sentry_multiple_requests
-    transaction = Sentry.start_transaction
-    Sentry.get_current_scope.set_span(transaction)
-
-    responses = HTTPX.get(build_uri("/status/200"), build_uri("/status/404"))
-    verify_status(responses[0], 200)
-    verify_status(responses[1], 404)
-    verify_spans(transaction, *responses)
-  end
-
-  private
-
-  def verify_spans(transaction, *responses, verb: nil, description: nil)
-    assert transaction.span_recorder.spans.count == responses.size + 1
-    assert transaction.span_recorder.spans[0] == transaction
-
-    response_spans = transaction.span_recorder.spans[1..-1]
-
-    responses.each_with_index do |response, idx|
-      request_span = response_spans[idx]
-      assert request_span.op == "httpx.client"
-      assert !request_span.start_timestamp.nil?
-      assert !request_span.timestamp.nil?
-      assert request_span.start_timestamp != request_span.timestamp
-      assert request_span.description == (description || "#{verb || "GET"} #{response.uri}")
-      assert request_span.data == { status: response.status }
-    end
-  end
-
-  def setup
-    super
-
-    mock_io = StringIO.new
-    mock_logger = Logger.new(mock_io)
-
-    Sentry.init do |config|
-      config.traces_sample_rate = 1.0
-      config.logger = mock_logger
-      config.dsn = DUMMY_DSN
-      config.transport.transport_class = Sentry::DummyTransport
-      # so the events will be sent synchronously for testing
-      config.background_worker_threads = 0
-    end
-  end
-
-  def origin
-    "https://#{httpbin}"
-  end
-end if RUBY_VERSION >= "2.4.0"

integration_tests/webmock_test.rb

@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 
-require "test_helper"
-require "support/http_helpers"
 require "webmock/minitest"
 require "httpx/adapters/webmock"
+require "test_helper"
+require "support/http_helpers"
 
 class WebmockTest < Minitest::Test
   include HTTPHelpers

lib/httpx.rb

@@ -67,3 +67,9 @@ end
 
 require "httpx/session"
 require "httpx/session_extensions"
+
+# load integrations when possible
+
+require "httpx/adapters/datadog" if defined?(DDTrace) || defined?(Datadog)
+require "httpx/adapters/sentry" if defined?(Sentry)
+require "httpx/adapters/webmock" if defined?(WebMock)

lib/httpx/connection.rb

@@ -316,7 +316,7 @@ module HTTPX
           # * the number of inflight requests
           # * the number of pending requests
           # * whether the write buffer has bytes (i.e. for close handshake)
-          if @pending.size.zero? && @inflight.zero? && @write_buffer.empty?
+          if @pending.empty? && @inflight.zero? && @write_buffer.empty?
             log(level: 3) { "NO MORE REQUESTS..." }
             return
           end
@@ -360,7 +360,7 @@ module HTTPX
             break if @state == :closing || @state == :closed
 
             # exit #consume altogether if all outstanding requests have been dealt with
-            return if @pending.size.zero? && @inflight.zero?
+            return if @pending.empty? && @inflight.zero?
           end unless ((ints = interests).nil? || ints == :w || @state == :closing) && !epiped
 
           #

lib/httpx/connection/http2.rb

@@ -365,7 +365,7 @@ module HTTPX
         ex.set_backtrace(caller)
         handle_error(ex)
       end
-      return unless is_connection_closed && @streams.size.zero?
+      return unless is_connection_closed && @streams.empty?
 
       emit(:close, is_connection_closed)
     end

lib/httpx/options.rb

@@ -100,7 +100,7 @@ module HTTPX
       end
 
       def def_option(optname, *args, &block)
-        if args.size.zero? && !block
+        if args.empty? && !block
           class_eval(<<-OUT, __FILE__, __LINE__ + 1)
             def option_#{optname}(v); v; end # def option_smth(v); v; end
           OUT

lib/httpx/plugins/compression.rb

@@ -30,7 +30,7 @@ module HTTPX
       module OptionsMethods
         def option_compression_threshold_size(value)
           bytes = Integer(value)
-          raise TypeError, ":expect_threshold_size must be positive" unless bytes.positive?
+          raise TypeError, ":compression_threshold_size must be positive" unless bytes.positive?
 
           bytes
         end
@@ -138,7 +138,7 @@ module HTTPX
         def each(&blk)
           return enum_for(__method__) unless blk
 
-          return deflate(&blk) if @buffer.size.zero?
+          return deflate(&blk) if @buffer.size.zero? # rubocop:disable Style/ZeroLengthPredicate
 
           @buffer.rewind
           @buffer.each(&blk)
@@ -152,7 +152,7 @@ module HTTPX
         private
 
         def deflate(&blk)
-          return unless @buffer.size.zero?
+          return unless @buffer.size.zero? # rubocop:disable Style/ZeroLengthPredicate
 
           @body.rewind
           @deflater.deflate(@body, @buffer, chunk_size: 16_384, &blk)

lib/httpx/plugins/compression/gzip.rb

@@ -56,7 +56,7 @@ module HTTPX
 
         class Inflater
           def initialize(bytesize)
-            @inflater = Zlib::Inflate.new(32 + Zlib::MAX_WBITS)
+            @inflater = Zlib::Inflate.new(Zlib::MAX_WBITS + 32)
             @bytesize = bytesize
             @buffer = nil
           end

lib/httpx/plugins/grpc/message.rb

@@ -57,7 +57,7 @@ module HTTPX
 
             yield data
 
-            message = message.byteslice((5 + size)..-1)
+            message = message.byteslice((size + 5)..-1)
           end
         end
 

lib/httpx/plugins/multipart/decoder.rb

@@ -5,10 +5,6 @@ require "delegate"
 
 module HTTPX::Plugins
   module Multipart
-    using HTTPX::RegexpExtensions unless Regexp.method_defined?(:match?)
-
-    CRLF = "\r\n"
-
     class FilePart < SimpleDelegator
       attr_reader :original_filename, :content_type
 
@@ -20,32 +16,14 @@ module HTTPX::Plugins
       end
     end
 
-    TOKEN = %r{[^\s()<>,;:\\"/\[\]?=]+}.freeze
-    VALUE = /"(?:\\"|[^"])*"|#{TOKEN}/.freeze
-    CONDISP = /Content-Disposition:\s*#{TOKEN}\s*/i.freeze
-    BROKEN_QUOTED = /^#{CONDISP}.*;\s*filename="(.*?)"(?:\s*$|\s*;\s*#{TOKEN}=)/i.freeze
-    BROKEN_UNQUOTED = /^#{CONDISP}.*;\s*filename=(#{TOKEN})/i.freeze
-    MULTIPART_CONTENT_TYPE = /Content-Type: (.*)#{CRLF}/ni.freeze
-    MULTIPART_CONTENT_DISPOSITION = /Content-Disposition:.*;\s*name=(#{VALUE})/ni.freeze
-    MULTIPART_CONTENT_ID = /Content-ID:\s*([^#{CRLF}]*)/ni.freeze
-    # Updated definitions from RFC 2231
-    ATTRIBUTE_CHAR = %r{[^ \t\v\n\r)(><@,;:\\"/\[\]?='*%]}.freeze
-    ATTRIBUTE = /#{ATTRIBUTE_CHAR}+/.freeze
-    SECTION = /\*[0-9]+/.freeze
-    REGULAR_PARAMETER_NAME = /#{ATTRIBUTE}#{SECTION}?/.freeze
-    REGULAR_PARAMETER = /(#{REGULAR_PARAMETER_NAME})=(#{VALUE})/.freeze
-    EXTENDED_OTHER_NAME = /#{ATTRIBUTE}\*[1-9][0-9]*\*/.freeze
-    EXTENDED_OTHER_VALUE = /%[0-9a-fA-F]{2}|#{ATTRIBUTE_CHAR}/.freeze
-    EXTENDED_OTHER_PARAMETER = /(#{EXTENDED_OTHER_NAME})=(#{EXTENDED_OTHER_VALUE}*)/.freeze
-    EXTENDED_INITIAL_NAME = /#{ATTRIBUTE}(?:\*0)?\*/.freeze
-    EXTENDED_INITIAL_VALUE = /[a-zA-Z0-9-]*'[a-zA-Z0-9-]*'#{EXTENDED_OTHER_VALUE}*/.freeze
-    EXTENDED_INITIAL_PARAMETER = /(#{EXTENDED_INITIAL_NAME})=(#{EXTENDED_INITIAL_VALUE})/.freeze
-    EXTENDED_PARAMETER = /#{EXTENDED_INITIAL_PARAMETER}|#{EXTENDED_OTHER_PARAMETER}/.freeze
-    DISPPARM = /;\s*(?:#{REGULAR_PARAMETER}|#{EXTENDED_PARAMETER})\s*/.freeze
-    RFC2183 = /^#{CONDISP}(#{DISPPARM})+$/i.freeze
-
     class Decoder
+      include HTTPX::Utils
+
+      CRLF = "\r\n"
       BOUNDARY_RE = /;\s*boundary=([^;]+)/i.freeze
+      MULTIPART_CONTENT_TYPE = /Content-Type: (.*)#{CRLF}/ni.freeze
+      MULTIPART_CONTENT_DISPOSITION = /Content-Disposition:.*;\s*name=(#{VALUE})/ni.freeze
+      MULTIPART_CONTENT_ID = /Content-ID:\s*([^#{CRLF}]*)/ni.freeze
       WINDOW_SIZE = 2 << 14
 
       def initialize(response)
@@ -102,7 +80,7 @@ module HTTPX::Plugins
             name = head[MULTIPART_CONTENT_ID, 1]
           end
 
-          filename = get_filename(head)
+          filename = HTTPX::Utils.get_filename(head)
 
           name = filename || +"#{content_type || "text/plain"}[]" if name.nil? || name.empty?
 
@@ -154,34 +132,6 @@ module HTTPX::Plugins
           raise Error, "parsing should have been over by now"
         end until @buffer.empty?
       end
-
-      def get_filename(head)
-        filename = nil
-        case head
-        when RFC2183
-          params = Hash[*head.scan(DISPPARM).flat_map(&:compact)]
-
-          if (filename = params["filename"])
-            filename = Regexp.last_match(1) if filename =~ /^"(.*)"$/
-          elsif (filename = params["filename*"])
-            encoding, _, filename = filename.split("'", 3)
-          end
-        when BROKEN_QUOTED, BROKEN_UNQUOTED
-          filename = Regexp.last_match(1)
-        end
-
-        return unless filename
-
-        filename = URI::DEFAULT_PARSER.unescape(filename) if filename.scan(/%.?.?/).all? { |s| /%[0-9a-fA-F]{2}/.match?(s) }
-
-        filename.scrub!
-
-        filename = filename.gsub(/\\(.)/, '\1') unless /\\[^\\"]/.match?(filename)
-
-        filename.force_encoding ::Encoding.find(encoding) if encoding
-
-        filename
-      end
     end
   end
 end

lib/httpx/plugins/retries.rb

@@ -26,7 +26,7 @@ module HTTPX
         ConnectionError,
         Connection::HTTP2::GoawayError,
       ].freeze
-      DEFAULT_JITTER = ->(interval) { interval * (0.5 * (1 + rand)) }
+      DEFAULT_JITTER = ->(interval) { interval * ((rand + 1) * 0.5) }
 
       if ENV.key?("HTTPX_NO_JITTER")
         def self.extra_options(options)

lib/httpx/response.rb

@@ -181,6 +181,12 @@ module HTTPX
         end
       end
 
+      def filename
+        return unless @headers.key?("content-disposition")
+
+        Utils.get_filename(@headers["content-disposition"])
+      end
+
       def to_s
         case @buffer
         when StringIO

lib/httpx/utils.rb

@@ -3,6 +3,12 @@
 module HTTPX
   module Utils
     using URIExtensions
+    using HTTPX::RegexpExtensions unless Regexp.method_defined?(:match?)
+
+    TOKEN = %r{[^\s()<>,;:\\"/\[\]?=]+}.freeze
+    VALUE = /"(?:\\"|[^"])*"|#{TOKEN}/.freeze
+    FILENAME_REGEX = /\s*filename=(#{VALUE})/.freeze
+    FILENAME_EXTENSION_REGEX = /\s*filename\*=(#{VALUE})/.freeze
 
     module_function
 
@@ -25,6 +31,30 @@ module HTTPX
       time - Time.now
     end
 
+    def get_filename(header, _prefix_regex = nil)
+      filename = nil
+      case header
+      when FILENAME_REGEX
+        filename = Regexp.last_match(1)
+        filename = Regexp.last_match(1) if filename =~ /^"(.*)"$/
+      when FILENAME_EXTENSION_REGEX
+        filename = Regexp.last_match(1)
+        encoding, _, filename = filename.split("'", 3)
+      end
+
+      return unless filename
+
+      filename = URI::DEFAULT_PARSER.unescape(filename) if filename.scan(/%.?.?/).all? { |s| /%[0-9a-fA-F]{2}/.match?(s) }
+
+      filename.scrub!
+
+      filename = filename.gsub(/\\(.)/, '\1') unless /\\[^\\"]/.match?(filename)
+
+      filename.force_encoding ::Encoding.find(encoding) if encoding
+
+      filename
+    end
+
     if RUBY_VERSION < "2.3"
 
       def to_uri(uri)

sig/plugins/multipart.rbs

@@ -68,8 +68,6 @@ module HTTPX
         def initialize: (Response response) -> void
 
         def parse: () -> void
-
-        def get_filename: (String head) -> String?
       end
 
       class FilePart #  < SimpleDelegator

sig/response.rbs

@@ -64,6 +64,8 @@ module HTTPX
       def each: () { (String) -> void } -> void
               | () -> Enumerable[String]
 
+      def filename: () -> String?
+
       def bytesize: () -> (Integer | Float)
       def empty?: () -> bool
       def copy_to: (String | File | _Writer destination) -> void

sig/utils.rbs

@@ -9,5 +9,7 @@ module HTTPX
     def self?.elapsed_time: (Integer | Float monotonic_time) -> Float
 
     def self?.to_uri: (generic_uri uri) -> URI::Generic
+
+    def self?.get_filename: (String header) -> String?
   end
 end

test/response_test.rb

@@ -138,6 +138,33 @@ class ResponseTest < Minitest::Test
     assert body.buffer.is_a?(Tempfile), "body should buffer to file after going over threshold"
   end
 
+  def test_response_body_filename
+    body = Response::Body.new(Response.new(request, 200, "2.0", {}), Options.new)
+    assert body.filename.nil?
+    body = Response::Body.new(Response.new(request, 200, "2.0", { "content-disposition" => "attachment;filename=test.csv" }), Options.new)
+    assert body.filename == "test.csv"
+    body = Response::Body.new(Response.new(request, 200, "2.0", { "content-disposition" => "attachment;filename=\"test.csv\"" }),
+                              Options.new)
+    assert body.filename == "test.csv"
+    body = Response::Body.new(Response.new(request, 200, "2.0", {
+                                             "content-disposition" => "inline; filename=ER886357.pdf; " \
+                                                                      "creation-date=9/17/2012 1:51:37 PM; " \
+                                                                      "modification-date=9/17/2012 1:51:37 PM; size=3718678",
+                                           }),
+                              Options.new)
+    assert body.filename == "ER886357.pdf"
+
+    body = Response::Body.new(Response.new(request, 200, "2.0", { "content-disposition" => "attachment; filename*=UTF-8''bar" }),
+                              Options.new)
+    assert body.filename == "bar"
+    body = Response::Body.new(Response.new(request, 200, "2.0", {
+                                             "content-disposition" => "inline; filename*=UTF-8''%c2%a3%20and%20%e2%82%ac%20rates.pdf",
+                                           }),
+                              Options.new)
+
+    assert body.filename == "£ and € rates.pdf"
+  end
+
   def test_response_decoders
     json_response = Response.new(request, 200, "2.0", { "content-type" => "application/json" })
     json_response << %({"a": "b"})