diff --git a/.rubocop.yml b/.rubocop.yml index 3522624a..bad8f003 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -28,7 +28,7 @@ AllCops: - 'test/extensions/response_pattern_match.rb' Metrics/ClassLength: - Max: 400 + Enabled: false Metrics/MethodLength: Max: 200 diff --git a/lib/httpx/connection.rb b/lib/httpx/connection.rb index 1792ef0a..c395f25e 100644 --- a/lib/httpx/connection.rb +++ b/lib/httpx/connection.rb @@ -102,8 +102,8 @@ module HTTPX # origin came from an ORIGIN frame, we're going to verify the hostname with the # SSL certificate (@origins.size == 1 || @origin == uri.origin || (@io && @io.verify_hostname(uri.host))) - ) || match_altsvcs?(uri) - ) && @options == options + ) && @options == options + ) || (match_altsvcs?(uri) && match_altsvc_options?(uri, options)) end def mergeable?(connection) @@ -162,6 +162,14 @@ module HTTPX 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 diff --git a/lib/httpx/plugins/response_cache/store.rb b/lib/httpx/plugins/response_cache/store.rb index 8f48d9e5..dbc0680a 100644 --- a/lib/httpx/plugins/response_cache/store.rb +++ b/lib/httpx/plugins/response_cache/store.rb @@ -37,6 +37,7 @@ module HTTPX::Plugins return unless request.headers.same_headers?(original_request.headers) else return unless vary.split(/ *, */).all? do |cache_field| + cache_field.downcase! !original_request.headers.key?(cache_field) || request.headers[cache_field] == original_request.headers[cache_field] end end diff --git a/lib/httpx/session.rb b/lib/httpx/session.rb index 06e4d716..0bbb4874 100644 --- a/lib/httpx/session.rb +++ b/lib/httpx/session.rb @@ -106,16 +106,21 @@ module HTTPX end def build_altsvc_connection(existing_connection, connections, alt_origin, origin, alt_params, options) + # do not allow security downgrades on altsvc negotiation + return if existing_connection.origin.scheme == "https" && alt_origin.scheme != "https" + altsvc = AltSvc.cached_altsvc_set(origin, alt_params.merge("origin" => alt_origin)) # altsvc already exists, somehow it wasn't advertised, probably noop return unless altsvc - connection = pool.find_connection(alt_origin, options) || build_connection(alt_origin, options) + 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 - set_connection_callbacks(connection, connections, options) + set_connection_callbacks(connection, connections, alt_options) log(level: 1) { "#{origin} alt-svc: #{alt_origin}" } diff --git a/sig/session.rbs b/sig/session.rbs index f2fc9190..93767ebd 100644 --- a/sig/session.rbs +++ b/sig/session.rbs @@ -33,7 +33,7 @@ module HTTPX def set_connection_callbacks: (Connection, Array[Connection], Options) -> void - def build_altsvc_connection: (Connection, Array[Connection], URI::Generic, String, Hash[String, String], 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? def build_requests: (verb | string, uri, options) -> Array[Request] | (Array[[verb | string, uri, options]], options) -> Array[Request] diff --git a/test/altsvc_test.rb b/test/altsvc_test.rb index 856b1bc3..9a385542 100644 --- a/test/altsvc_test.rb +++ b/test/altsvc_test.rb @@ -50,4 +50,18 @@ class AltSvcTest < Minitest::Test ["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 + + def test_altsvc_clear_cache + AltSvc.cached_altsvc_set("http://www.example.com", { "origin" => "http://alt.example.com", "ma" => 2 }) + entries = AltSvc.cached_altsvc("http://www.example.com") + assert !entries.empty? + + req = Request.new(:get, "http://www.example.com/") + res = Response.new(req, 200, "2.0", { "alt-svc" => "clear" }) + + AltSvc.emit(req, res) + + entries = AltSvc.cached_altsvc("http://www.example.com") + assert entries.empty? + end end diff --git a/test/response_cache_store_test.rb b/test/response_cache_store_test.rb new file mode 100644 index 00000000..2f8f9c3c --- /dev/null +++ b/test/response_cache_store_test.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require_relative "test_helper" +require "httpx/plugins/response_cache/store" + +class ResponseCacheStoreTest < Minitest::Test + include HTTPX + + def test_store_cache + request = Request.new(:get, "http://example.com/") + response = cached_response(request) + + assert store.lookup(request.uri) == response + assert store.cached?(request.uri) + + request2 = Request.new(:get, "http://example.com/", headers: { "accept" => "text/plain" }) + assert store.lookup(request2.uri) == response + end + + def test_prepare_vary + request = Request.new(:get, "http://example.com/", headers: { "accept" => "text/plain" }) + cached_response(request, { "vary" => "Accept" }) + + request2 = Request.new(:get, "http://example.com/", headers: { "accept" => "text/html" }) + store.prepare(request2) + assert !request2.headers.key?("if-none-match") + request3 = Request.new(:get, "http://example.com/", headers: { "accept" => "text/plain" }) + store.prepare(request3) + assert request3.headers.key?("if-none-match") + request4 = Request.new(:get, "http://example.com/", headers: { "accept" => "text/plain", "user-agent" => "Linux Bowser" }) + store.prepare(request4) + assert request4.headers.key?("if-none-match") + end + + def test_prepare_vary_asterisk + request = Request.new(:get, "http://example.com/", headers: { "accept" => "text/plain" }) + cached_response(request, { "vary" => "*" }) + + request2 = Request.new(:get, "http://example.com/", headers: { "accept" => "text/html" }) + store.prepare(request2) + assert !request2.headers.key?("if-none-match") + request3 = Request.new(:get, "http://example.com/", headers: { "accept" => "text/plain" }) + store.prepare(request3) + assert request3.headers.key?("if-none-match") + request4 = Request.new(:get, "http://example.com/", headers: { "accept" => "text/plain", "user-agent" => "Linux Bowser" }) + store.prepare(request4) + assert !request4.headers.key?("if-none-match") + end + + private + + def store + @store ||= Plugins::ResponseCache::Store.new + end + + def cached_response(request, extra_headers = {}) + response = Response.new(request, 200, "2.0", { "etag" => "ETAG" }.merge(extra_headers)) + store.cache(request.uri, response) + response + end +end diff --git a/test/support/requests/plugins/response_cache.rb b/test/support/requests/plugins/response_cache.rb index 8fdfa8c8..e9f2fbe9 100644 --- a/test/support/requests/plugins/response_cache.rb +++ b/test/support/requests/plugins/response_cache.rb @@ -42,25 +42,6 @@ module Requests # assert expired.body != uncached.body end - - def test_plugin_response_cache_vary - skip - return unless origin.start_with?("https://") - - vary_uri = "https://github.com/HoneyryderChuck/httpx" - - HTTPX.plugin(:response_cache).wrap do |cache_client| - uncached = cache_client.get(vary_uri) - verify_status(uncached, 200) - cached = cache_client.get(vary_uri) - verify_status(cached, 304) - - assert uncached.body == cached.body - - uncached = cache_client.get(vary_uri, headers: { "accept" => "text/plain" }) - verify_status(uncached, 200) - end - end end end end