Merge branch 'fix-altsvc' into 'master'

Fix ALTSVC parsing

See merge request honeyryderchuck/httpx!44

docker-compose.yml

@@ -10,7 +10,6 @@ services:
       - HTTPX_SOCKS4A_PROXY=socks4a://socksproxy:8080
       - HTTPX_SOCKS5_PROXY=socks5://socksproxy:8080
       - HTTPX_SSH_PROXY=ssh://sshproxy:22
-      - PARALLEL=1
       - N=6 # minitest workers
       - CI=1
       - JEKYLL_ENV=production

lib/httpx/altsvc.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require "strscan"
+
 module HTTPX
   module AltSvc
     @altsvc_mutex = Mutex.new
@@ -49,9 +51,18 @@ module HTTPX
     def parse(altsvc)
       return enum_for(__method__, altsvc) unless block_given?
 
-      alt_origins, *alt_params = altsvc.split(/ *; */)
-      alt_params = Hash[alt_params.map { |field| field.split("=") }]
-      alt_origins.split(/ *, */).each do |alt_origin|
+      scanner = StringScanner.new(altsvc)
+      until scanner.eos?
+        alt_origin = scanner.scan(/[^=]+=("[^"]+"|[^;,]+)/)
+
+        alt_params = []
+        loop do
+          alt_param = scanner.scan(/[^=]+=("[^"]+"|[^;,]+)/)
+          alt_params << alt_param.strip if alt_param
+          scanner.skip(/;/)
+          break if scanner.eos? || scanner.scan(/ *, */)
+        end
+        alt_params = Hash[alt_params.map { |field| field.split("=") }]
         yield(parse_altsvc_origin(alt_origin), alt_params)
       end
     end

lib/httpx/connection.rb

@@ -46,8 +46,6 @@ module HTTPX
 
     attr_reader :origin, :state, :pending, :options
 
-    attr_reader :timeout
-
     def initialize(type, uri, options)
       @type = type
       @origins = [uri.origin]
@@ -188,7 +186,6 @@ module HTTPX
     end
 
     def call
-      @timeout = @timeout_threshold
       case @state
       when :closed
         return
@@ -202,6 +199,14 @@ module HTTPX
       nil
     end
 
+    def timeout
+      return @timeout if defined?(@timeout)
+
+      return @options.timeout.connect_timeout if @state == :idle
+
+      @options.timeout.operation_timeout
+    end
+
     private
 
     def consume
@@ -289,8 +294,8 @@ module HTTPX
           transition(:open)
         end
       end
-      parser.on(:timeout) do |timeout|
-        @timeout = timeout
+      parser.on(:timeout) do |tout|
+        @timeout = tout
       end
       parser.on(:error) do |request, ex|
         case ex
@@ -307,8 +312,6 @@ module HTTPX
       case nextstate
       when :idle
         @error = nil
-        @timeout_threshold = @options.timeout.connect_timeout
-        @timeout = @timeout_threshold
       when :open
         return if @state == :closed
 
@@ -316,8 +319,6 @@ module HTTPX
         return unless @io.connected?
 
         send_pending
-        @timeout_threshold = @options.timeout.operation_timeout
-        @timeout = @timeout_threshold
         emit(:open)
       when :closing
         return unless @state == :open
@@ -327,11 +328,10 @@ module HTTPX
 
         @io.close
         @read_buffer.clear
+        remove_instance_variable(:@timeout) if defined?(@timeout)
       when :already_open
         nextstate = :open
         send_pending
-        @timeout_threshold = @options.timeout.operation_timeout
-        @timeout = @timeout_threshold
       end
       @state = nextstate
     rescue Errno::EHOSTUNREACH

lib/httpx/timeout.rb

@@ -81,8 +81,11 @@ module HTTPX
       return reset_timer unless @started
 
       @time_left -= (Process.clock_gettime(Process::CLOCK_MONOTONIC) - @started)
-      raise TotalTimeoutError.new(@total_timeout, "Timed out after #{@total_timeout} seconds") if no_time_left?
-
+      if no_time_left?
+        reset_counter
+        raise TotalTimeoutError.new(@total_timeout, "Timed out after #{@total_timeout} seconds")
+      end
+    ensure
       reset_timer
     end
   end

test/altsvc_test.rb

@@ -5,14 +5,38 @@ require_relative "test_helper"
 class AltSvcTest < Minitest::Test
   include HTTPX
 
-  def test_parse
+  def test_altsvc_parse_svc
     assert [["h2=alt.example.com", {}]], AltSvc.parse("h2=alt.example.com").to_a
+  end
+
+  def test_altsvc_parse_svc_with_port
     assert [["h2=alt.example.com:8000", {}]], AltSvc.parse("h2=\"alt.example.com:8000\"").to_a
+  end
+
+  def test_altsvc_parse_svcs
     assert [["h2=alt.example.com:8000", {}], ["h2=:8000", {}]],
            AltSvc.parse("h2=\"alt.example.com:8000\", h2=\":443\"").to_a
+  end
+
+  def test_altsvc_parse_svc_prop
     assert [["h2=alt.example.com:8000'", { "ma" => "60" }]],
            AltSvc.parse("h2=\"alt.example.com:8000\"; ma=60").to_a
+  end
+
+  def test_altsvc_parse_svc_props
     assert [["h2=alt.example.com:8000", { "persist" => "1" }]],
            AltSvc.parse("h2=\"alt.example.com:8000\"; ma=60; persist=1").to_a
   end
+
+  def test_altsvc_parse_svc_with_versions
+    assert [["quic=:443", { "ma" => "2592000", "v" => "46,43,39" }]],
+           AltSvc.parse("quic=\":443\"; ma=2592000; v=\"46,43,39\"").to_a
+  end
+
+  def test_altsvc_parse_svcs_with_props
+    assert [["quic=:443", { "ma" => "2592000", "v" => "46,43" }],
+            ["h3-Q046=:443", { "ma" => "2592000" }],
+            ["h3-Q043=:443", { "ma" => "2592000" }]],
+           AltSvc.parse("quic=\":443\"; ma=2592000; v=\"46,43\",h3-Q046=\":443\"; ma=2592000,h3-Q043=\":443\"; ma=2592000").to_a
+  end
 end