Merge branch 'options-improvements' into 'master'

adding new Options#merge implementation

See merge request os85/httpx!297
This commit is contained in:
HoneyryderChuck 2023-11-29 17:17:12 +00:00
commit 4809e1d0d0
15 changed files with 268 additions and 114 deletions

View File

@ -4,6 +4,58 @@ require "strscan"
module HTTPX
module AltSvc
# makes connections able to accept requests destined to primary service.
module ConnectionMixin
using URIExtensions
def send(request)
request.headers["alt-used"] = @origin.authority if @parser && !@write_buffer.full? && match_altsvcs?(request.uri)
super
end
def match?(uri, options)
return false if !used? && (@state == :closing || @state == :closed)
match_altsvcs?(uri) && match_altsvc_options?(uri, options)
end
private
# checks if this is connection is an alternative service of
# +uri+
def match_altsvcs?(uri)
@origins.any? { |origin| altsvc_match?(uri, origin) } ||
AltSvc.cached_altsvc(@origin).any? do |altsvc|
origin = altsvc["origin"]
altsvc_match?(origin, uri.origin)
end
end
def match_altsvc_options?(uri, options)
return @options == options unless @options.ssl.all? do |k, v|
v == (k == :hostname ? uri.host : options.ssl[k])
end
@options.options_equals?(options, Options::REQUEST_BODY_IVARS + %i[@ssl])
end
def altsvc_match?(uri, other_uri)
other_uri = URI(other_uri)
uri.origin == other_uri.origin || begin
case uri.scheme
when "h2"
(other_uri.scheme == "https" || other_uri.scheme == "h2") &&
uri.host == other_uri.host &&
uri.port == other_uri.port
else
false
end
end
end
end
@altsvc_mutex = Thread::Mutex.new
@altsvcs = Hash.new { |h, k| h[k] = [] }
@ -99,7 +151,10 @@ module HTTPX
end
def parse_altsvc_origin(alt_proto, alt_origin)
alt_scheme = parse_altsvc_scheme(alt_proto) or return
alt_scheme = parse_altsvc_scheme(alt_proto)
return unless alt_scheme
alt_origin = alt_origin[1..-2] if alt_origin.start_with?("\"") && alt_origin.end_with?("\"")
URI.parse("#{alt_scheme}://#{alt_origin}")

View File

@ -93,15 +93,13 @@ module HTTPX
return false if !used? && (@state == :closing || @state == :closed)
(
(
@origins.include?(uri.origin) &&
# if there is more than one origin to match, it means that this connection
# was the result of coalescing. To prevent blind trust in the case where the
# origin came from an ORIGIN frame, we're going to verify the hostname with the
# SSL certificate
(@origins.size == 1 || @origin == uri.origin || (@io.is_a?(SSL) && @io.verify_hostname(uri.host)))
) && @options == options
) || (match_altsvcs?(uri) && match_altsvc_options?(uri, options))
@origins.include?(uri.origin) &&
# if there is more than one origin to match, it means that this connection
# was the result of coalescing. To prevent blind trust in the case where the
# origin came from an ORIGIN frame, we're going to verify the hostname with the
# SSL certificate
(@origins.size == 1 || @origin == uri.origin || (@io.is_a?(SSL) && @io.verify_hostname(uri.host)))
) && @options == options
end
def expired?
@ -163,24 +161,6 @@ module HTTPX
end
end
# checks if this is connection is an alternative service of
# +uri+
def match_altsvcs?(uri)
@origins.any? { |origin| uri.altsvc_match?(origin) } ||
AltSvc.cached_altsvc(@origin).any? do |altsvc|
origin = altsvc["origin"]
origin.altsvc_match?(uri.origin)
end
end
def match_altsvc_options?(uri, options)
return @options == options unless @options.ssl[:hostname] == uri.host
dup_options = @options.merge(ssl: { hostname: nil })
dup_options.ssl.delete(:hostname)
dup_options == options
end
def connecting?
@state == :idle
end
@ -258,8 +238,6 @@ module HTTPX
def send(request)
if @parser && !@write_buffer.full?
request.headers["alt-used"] = @origin.authority if match_altsvcs?(request.uri)
if @response_received_at && @keep_alive_timeout &&
Utils.elapsed_time(@response_received_at) > @keep_alive_timeout
# when pushing a request into an existing connection, we have to check whether there

View File

@ -54,21 +54,6 @@ module HTTPX
def origin
"#{scheme}://#{authority}"
end unless URI::HTTP.method_defined?(:origin)
def altsvc_match?(uri)
uri = URI.parse(uri)
origin == uri.origin || begin
case scheme
when "h2"
(uri.scheme == "https" || uri.scheme == "h2") &&
host == uri.host &&
(port || default_port) == (uri.port || uri.default_port)
else
false
end
end
end
end
end
end

View File

@ -228,26 +228,72 @@ module HTTPX
OUT
end
REQUEST_IVARS = %i[@params @form @xml @json @body].freeze
private_constant :REQUEST_IVARS
REQUEST_BODY_IVARS = %i[@headers @params @form @xml @json @body].freeze
def ==(other)
ivars = instance_variables | other.instance_variables
super || options_equals?(other)
end
def options_equals?(other, ignore_ivars = REQUEST_BODY_IVARS)
# headers and other request options do not play a role, as they are
# relevant only for the request.
ivars = instance_variables - ignore_ivars
other_ivars = other.instance_variables - ignore_ivars
return false if ivars.size != other_ivars.size
return false if ivars.sort != other_ivars.sort
ivars.all? do |ivar|
case ivar
when :@headers
# currently, this is used to pick up an available matching connection.
# the headers do not play a role, as they are relevant only for the request.
true
when *REQUEST_IVARS
true
else
instance_variable_get(ivar) == other.instance_variable_get(ivar)
end
instance_variable_get(ivar) == other.instance_variable_get(ivar)
end
end
OTHER_LOOKUP = ->(obj, k, ivar_map) {
case obj
when Hash
obj[ivar_map[k]]
else
obj.instance_variable_get(k)
end
}
def merge(other)
ivar_map = nil
other_ivars = case other
when Hash
ivar_map = other.keys.to_h { |k| [:"@#{k}", k] }
ivar_map.keys
else
other.instance_variables
end
return self if other_ivars.empty?
return self if other_ivars.all? { |ivar| instance_variable_get(ivar) == OTHER_LOOKUP[other, ivar, ivar_map] }
opts = dup
other_ivars.each do |ivar|
v = OTHER_LOOKUP[other, ivar, ivar_map]
unless v
opts.instance_variable_set(ivar, v)
next
end
v = opts.__send__(:"option_#{ivar[1..-1]}", v)
orig_v = instance_variable_get(ivar)
v = orig_v.merge(v) if orig_v.respond_to?(:merge) && v.respond_to?(:merge)
opts.instance_variable_set(ivar, v)
end
opts
end
def merge2(other)
raise ArgumentError, "#{other} is not a valid set of options" unless other.respond_to?(:to_hash)
h2 = other.to_hash
@ -274,10 +320,40 @@ module HTTPX
end
end
def initialize_dup(other)
instance_variables.each do |ivar|
instance_variable_set(ivar, other.instance_variable_get(ivar).dup)
def extend_with_plugin_classes(pl)
if defined?(pl::RequestMethods) || defined?(pl::RequestClassMethods)
@request_class = @request_class.dup
@request_class.__send__(:include, pl::RequestMethods) if defined?(pl::RequestMethods)
@request_class.extend(pl::RequestClassMethods) if defined?(pl::RequestClassMethods)
end
if defined?(pl::ResponseMethods) || defined?(pl::ResponseClassMethods)
@response_class = @response_class.dup
@response_class.__send__(:include, pl::ResponseMethods) if defined?(pl::ResponseMethods)
@response_class.extend(pl::ResponseClassMethods) if defined?(pl::ResponseClassMethods)
end
if defined?(pl::HeadersMethods) || defined?(pl::HeadersClassMethods)
@headers_class = @headers_class.dup
@headers_class.__send__(:include, pl::HeadersMethods) if defined?(pl::HeadersMethods)
@headers_class.extend(pl::HeadersClassMethods) if defined?(pl::HeadersClassMethods)
end
if defined?(pl::RequestBodyMethods) || defined?(pl::RequestBodyClassMethods)
@request_body_class = @request_body_class.dup
@request_body_class.__send__(:include, pl::RequestBodyMethods) if defined?(pl::RequestBodyMethods)
@request_body_class.extend(pl::RequestBodyClassMethods) if defined?(pl::RequestBodyClassMethods)
end
if defined?(pl::ResponseBodyMethods) || defined?(pl::ResponseBodyClassMethods)
@response_body_class = @response_body_class.dup
@response_body_class.__send__(:include, pl::ResponseBodyMethods) if defined?(pl::ResponseBodyMethods)
@response_body_class.extend(pl::ResponseBodyClassMethods) if defined?(pl::ResponseBodyClassMethods)
end
if defined?(pl::ConnectionMethods)
@connection_class = @connection_class.dup
@connection_class.__send__(:include, pl::ConnectionMethods)
end
return unless defined?(pl::OptionsMethods)
@options_class = @options_class.dup
@options_class.__send__(:include, pl::OptionsMethods)
end
private

View File

@ -71,21 +71,31 @@ module HTTPX
end
module OptionsMethods
def do_initialize(*)
super
def option_headers(*)
value = super
return unless @headers.key?("cookie")
merge_cookie_in_jar(value.delete("cookie"), @cookies) if defined?(@cookies) && value.key?("cookie")
@headers.delete("cookie").each do |ck|
ck.split(/ *; */).each do |cookie|
name, value = cookie.split("=", 2)
@cookies.add(Cookie.new(name, value))
end
end
value
end
def option_cookies(value)
value.is_a?(Jar) ? value : Jar.new(value)
jar = value.is_a?(Jar) ? value : Jar.new(value)
merge_cookie_in_jar(@headers.delete("cookie"), jar) if defined?(@headers) && @headers.key?("cookie")
jar
end
private
def merge_cookie_in_jar(cookies, jar)
cookies.each do |ck|
ck.split(/ *; */).each do |cookie|
name, value = cookie.split("=", 2)
jar.add(Cookie.new(name, value))
end
end
end
end
end

View File

@ -261,14 +261,14 @@ module HTTPX
headers["grpc-timeout"] = "#{deadline}m"
end
headers = headers.merge(metadata) if metadata
headers = headers.merge(metadata.transform_keys(&:to_s)) if metadata
# prepare compressor
compression = @options.grpc_compression == true ? "gzip" : @options.grpc_compression
headers["grpc-encoding"] = compression if compression
headers.merge!(@options.call_credentials.call) if @options.call_credentials
headers.merge!(@options.call_credentials.call.transform_keys(&:to_s)) if @options.call_credentials
build_request("POST", uri, headers: headers, body: input)
end

View File

@ -194,22 +194,16 @@ module HTTPX
alt_options = options.merge(ssl: options.ssl.merge(hostname: URI(origin).host))
connection = pool.find_connection(alt_origin, alt_options) || build_connection(alt_origin, alt_options)
# advertised altsvc is the same origin being used, ignore
return if connection == existing_connection
connection.extend(AltSvc::ConnectionMixin) unless connection.is_a?(AltSvc::ConnectionMixin)
set_connection_callbacks(connection, connections, alt_options)
log(level: 1) { "#{origin} alt-svc: #{alt_origin}" }
# get uninitialized requests
# incidentally, all requests will be re-routed to the first
# advertised alt-svc, which incidentally follows the spec.
existing_connection.purge_pending do |request|
request.origin == origin &&
request.state == :idle &&
!request.headers.key?("alt-used")
end
connection.merge(existing_connection)
existing_connection.terminate
connection
@ -361,19 +355,8 @@ module HTTPX
extend(pl::ClassMethods) if defined?(pl::ClassMethods)
opts = @default_options
opts.request_class.__send__(:include, pl::RequestMethods) if defined?(pl::RequestMethods)
opts.request_class.extend(pl::RequestClassMethods) if defined?(pl::RequestClassMethods)
opts.response_class.__send__(:include, pl::ResponseMethods) if defined?(pl::ResponseMethods)
opts.response_class.extend(pl::ResponseClassMethods) if defined?(pl::ResponseClassMethods)
opts.headers_class.__send__(:include, pl::HeadersMethods) if defined?(pl::HeadersMethods)
opts.headers_class.extend(pl::HeadersClassMethods) if defined?(pl::HeadersClassMethods)
opts.request_body_class.__send__(:include, pl::RequestBodyMethods) if defined?(pl::RequestBodyMethods)
opts.request_body_class.extend(pl::RequestBodyClassMethods) if defined?(pl::RequestBodyClassMethods)
opts.response_body_class.__send__(:include, pl::ResponseBodyMethods) if defined?(pl::ResponseBodyMethods)
opts.response_body_class.extend(pl::ResponseBodyClassMethods) if defined?(pl::ResponseBodyClassMethods)
opts.connection_class.__send__(:include, pl::ConnectionMethods) if defined?(pl::ConnectionMethods)
opts.extend_with_plugin_classes(pl)
if defined?(pl::OptionsMethods)
opts.options_class.__send__(:include, pl::OptionsMethods)
(pl::OptionsMethods.instance_methods - Object.instance_methods).each do |meth|
opts.options_class.method_added(meth)

33
sig/altsvc.rbs Normal file
View File

@ -0,0 +1,33 @@
module HTTPX
module AltSvc
module ConnectionMixin
def send: (Request request) -> void
def match?: (URI::Generic uri, Options options) -> bool
private
def match_altsvcs?: (URI::Generic uri) -> bool
def match_altsvc_options?: (URI::Generic uri, Options options) -> bool
end
type altsvc_params = Hash[String, untyped]
def self?.cached_altsvc: (String origin) -> Array[altsvc_params]
def self?.cached_altsvc_set: (String origin, altsvc_params) -> void
def self?.lookup: (String origin, Integer | Float ttl) -> Array[altsvc_params]
def self?.emit: (Request request, response response) { (http_uri alt_origin, String origin, altsvc_params alt_params) -> void } -> void
def self?.parse: (String altsvc) { (http_uri alt_origin, altsvc_params alt_params) -> void } -> void
| (String altsvc) -> Enumerable[[http_uri, altsvc_params]]
def self?.parse_altsvc_scheme: (String alt_proto) -> String?
def self.parse_altsvc_origin: (string alt_proto, String alt_origin) -> http_uri?
end
end

View File

@ -44,25 +44,23 @@ module HTTPX
def addresses: () -> Array[ipaddr]?
def addresses=: (Array[ipaddr]) -> void
def addresses=: (Array[ipaddr] addresses) -> void
def send: (Request request) -> void
def match?: (URI::Generic uri, Options options) -> bool
def expired?: () -> boolish
def mergeable?: (Connection) -> bool
def mergeable?: (Connection connection) -> bool
def coalescable?: (Connection) -> bool
def coalescable?: (Connection connection) -> bool
def create_idle: (?Hash[Symbol, untyped] options) -> Connection
def merge: (Connection) -> void
def merge: (Connection connection) -> void
def purge_pending: () { (Request) -> void } -> void
def match_altsvcs?: (URI::Generic uri) -> bool
def match_altsvc_options?: (URI::Generic uri, Options options) -> bool
def purge_pending: () { (Request request) -> void } -> void
def connecting?: () -> bool
@ -80,8 +78,6 @@ module HTTPX
def reset: () -> void
def send: (Request) -> void
def timeout: () -> Numeric?
def idling: () -> void
@ -114,15 +110,15 @@ module HTTPX
def set_parser_callbacks: (HTTP1 | HTTP2 parser) -> void
def transition: (Symbol) -> void
def transition: (Symbol nextstate) -> void
def handle_transition: (Symbol) -> void
def handle_transition: (Symbol nextstate) -> void
def build_socket: (?Array[ipaddr]? addrs) -> (TCP | SSL | UNIX)
def on_error: (HTTPX::TimeoutError | Error | StandardError) -> void
def on_error: (HTTPX::TimeoutError | Error | StandardError error) -> void
def handle_error: (StandardError) -> void
def handle_error: (StandardError error) -> void
def purge_after_closed: () -> void

View File

@ -11,6 +11,7 @@ module HTTPX
SETTINGS_TIMEOUT: Integer
CLOSE_HANDSHAKE_TIMEOUT: Integer
DEFAULT_OPTIONS: Hash[Symbol, untyped]
REQUEST_BODY_IVARS: Array[Symbol]
type timeout_type = :connect_timeout | :settings_timeout | :close_handshake_timeout | :operation_timeout | :keep_alive_timeout | :read_timeout | :write_timeout | :request_timeout
type timeout = Hash[timeout_type, Numeric?]
@ -121,10 +122,16 @@ module HTTPX
# ip_families
attr_reader ip_families: Array[ip_family]
def ==: (untyped other) -> bool
def ==: (Options other) -> bool
def options_equals?: (Options other, ?Array[Symbol] ignore_ivars) -> bool
def merge: (_ToHash[Symbol, untyped] other) -> instance
def to_hash: () -> Hash[Symbol, untyped]
def extend_with_plugin_classes: (Module pl) -> void
private
REQUEST_IVARS: Array[Symbol]

View File

@ -5,6 +5,8 @@ module HTTPX
interface _CookieOptions
def cookies: () -> Jar?
def merge_cookie_in_jar: (Array[String] cookies, Jar jar) -> void
end
def self.extra_options: (Options) -> (Options & _CookieOptions)

View File

@ -33,7 +33,7 @@ module HTTPX
def set_connection_callbacks: (Connection connection, Array[Connection] connections, Options options) -> void
def build_altsvc_connection: (Connection existing_connection, Array[Connection] connections, URI::Generic alt_origin, String origin, Hash[String, String] alt_params, Options options) -> Connection?
def build_altsvc_connection: (Connection existing_connection, Array[Connection] connections, URI::Generic alt_origin, String origin, Hash[String, String] alt_params, Options options) -> (Connection & AltSvc::ConnectionMixin)?
def build_requests: (verb, uri, options) -> Array[Request]
| (Array[[verb, uri, options]], options) -> Array[Request]

View File

@ -65,7 +65,7 @@ class AltSvcTest < Minitest::Test
req = Request.new("GET", "http://www.example-clear-cache.com/")
res = Response.new(req, 200, "2.0", { "alt-svc" => "clear" })
AltSvc.emit(req, res)
AltSvc.emit(req, res) {}
entries = AltSvc.cached_altsvc("http://www.example-clear-cache.com")
assert entries.empty?

View File

@ -33,26 +33,48 @@ class OptionsTest < Minitest::Test
assert opt2.headers.to_a == [%w[accept */*]], "headers are unexpected"
end
def test_options_merge
def test_options_merge_hash
opts = Options.new(body: "fat")
merged_opts = opts.merge(body: "thin")
assert merged_opts.body == "thin", "parameter hasn't been merged"
assert opts.body == "fat", "original parameter has been mutated after merge"
assert !opts.equal?(merged_opts), "merged options should be a different object"
end
def test_options_merge_options
opts = Options.new(body: "fat")
merged_opts2 = opts.merge(Options.new(body: "short"))
assert opts.body == "fat", "original parameter has been mutated after merge"
assert merged_opts2.body == "short", "options parameter hasn't been merged"
assert !opts.equal?(merged_opts2), "merged options should be a different object"
end
def test_options_merge_options_empty_hash
opts = Options.new(body: "fat")
merged_opts3 = opts.merge({})
assert opts.equal?(merged_opts3), "merged options should be the same object"
end
def test_options_merge_same_options
opts = Options.new(body: "fat")
merged_opts4 = opts.merge({ body: "fat" })
assert opts.equal?(merged_opts4), "merged options should be the same object"
merged_opts5 = opts.merge(Options.new(body: "fat"))
assert opts.equal?(merged_opts5), "merged options should be the same object"
end
def test_options_merge_origin_uri
opts = Options.new(origin: "http://example.com")
merged_opts = opts.merge(Options.new(origin: "http://example2.com"))
assert merged_opts.origin == URI("http://example2.com")
merged_opts = opts.merge({ origin: "http://example2.com" })
assert merged_opts.origin == URI("http://example2.com")
end
def test_options_merge_attributes_match
foo = Options.new(
:form => { :foo => "foo" },
:headers => { :accept => "json", :foo => "foo" },
@ -129,4 +151,11 @@ class OptionsTest < Minitest::Test
opts = Options.new
assert opts.to_hash.is_a?(Hash)
end
def test_options_equals
opts = Options.new(origin: "http://example.com")
assert opts == Options.new(origin: "http://example.com")
assert Options.new(origin: "http://example.com", headers: { "foo" => "bar" }) == Options.new(origin: "http://example.com")
assert Options.new(json: { "foo" => "bar" }) == Options.new
end
end

View File

@ -6,7 +6,7 @@ module Requests
altsvc_host = ENV["HTTPBIN_ALTSVC_HOST"]
altsvc_origin = origin(altsvc_host)
HTTPX.wrap do |http|
HTTPX.plugin(SessionWithPool).wrap do |http|
altsvc_uri = build_uri("/get", altsvc_origin)
response = http.get(altsvc_uri)
verify_status(response, 200)