From 26f5a1a3980a1ecba3498c03defd8f5aeaa63ee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?George=20Guimar=C3=A3es?= Date: Tue, 15 Sep 2026 17:15:55 -0300 Subject: [PATCH 1/3] fix: Parse JSON cache entries without create_additions The default serializer read entries back with JSON.load, which honours a json_class key and calls json_create on that class. Response headers are stored verbatim in the entry, so an origin server could name any loaded class in a single header and have it instantiated on the next cache hit, through both strategies. JSON.parse builds plain Ruby objects only. Passing create_additions: false to JSON.load is not a fix: the keyword binds to the proc positional parameter and the option never arrives. Cache keys are unchanged, so existing entries stay readable and become inert. --- lib/faraday/http_cache/strategies/base_strategy.rb | 11 +++++++++-- spec/spec_helper.rb | 1 + spec/strategies/by_url_spec.rb | 14 ++++++++++++++ spec/strategies/by_vary_spec.rb | 14 ++++++++++++++ spec/support/json_gadget.rb | 14 ++++++++++++++ 5 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 spec/support/json_gadget.rb diff --git a/lib/faraday/http_cache/strategies/base_strategy.rb b/lib/faraday/http_cache/strategies/base_strategy.rb index 2f56dcc..a96de48 100644 --- a/lib/faraday/http_cache/strategies/base_strategy.rb +++ b/lib/faraday/http_cache/strategies/base_strategy.rb @@ -29,7 +29,9 @@ class BaseStrategy # @option options [Faraday::HttpCache::MemoryStore, nil] :store - a cache # store object that should respond to 'read', 'write', and 'delete'. # @option options [#dump#load] :serializer - an object that should - # respond to 'dump' and 'load'. + # respond to 'dump' and 'load'. 'load' must never instantiate classes + # named by the data, since the cached entries contain response headers + # sent by the origin server. # @option options [Logger, nil] :logger - an object to be used to emit warnings. def initialize(options = {}) @cache = options[:store] || Faraday::HttpCache::MemoryStore.new @@ -80,7 +82,12 @@ def deserialize_entry(*objects) end def deserialize_object(object) - @serializer.load(object).transform_keys(&:to_sym) + # JSON.load enables create_additions, so a `json_class` key in the + # entry would instantiate that class. Response headers are stored + # verbatim, which lets an origin server plant such a key. JSON.parse + # only ever builds plain Ruby objects. + loaded = @serializer.equal?(::JSON) ? ::JSON.parse(object) : @serializer.load(object) + loaded.transform_keys(&:to_sym) end def warn(message) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2611303..5d101e8 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -16,6 +16,7 @@ require 'support/test_app' require 'support/test_server' +require 'support/json_gadget' server = TestServer.new diff --git a/spec/strategies/by_url_spec.rb b/spec/strategies/by_url_spec.rb index e8fdbe9..4530094 100644 --- a/spec/strategies/by_url_spec.rb +++ b/spec/strategies/by_url_spec.rb @@ -16,6 +16,20 @@ let(:strategy) { described_class.new(store: cache) } subject { strategy } + describe 'deserializing entries' do + let(:response) { double(serializable_hash: { response_headers: { 'json_class' => 'JsonGadget' } }) } + + before { JsonGadget.invocations.clear } + + it 'never instantiates classes named by the cached data' do + strategy.write(request, response) + cached = strategy.read(request) + + expect(JsonGadget.invocations).to be_empty + expect(cached.payload[:response_headers]['json_class']).to eq('JsonGadget') + end + end + describe 'Cache configuration' do it 'uses a MemoryStore by default' do expect(Faraday::HttpCache::MemoryStore).to receive(:new).and_call_original diff --git a/spec/strategies/by_vary_spec.rb b/spec/strategies/by_vary_spec.rb index 54a61a8..32dfd6c 100644 --- a/spec/strategies/by_vary_spec.rb +++ b/spec/strategies/by_vary_spec.rb @@ -23,6 +23,20 @@ let(:strategy) { described_class.new(store: cache) } subject { strategy } + describe 'deserializing entries' do + let(:response_payload) { { response_headers: { 'Vary' => vary, 'json_class' => 'JsonGadget' } } } + + before { JsonGadget.invocations.clear } + + it 'never instantiates classes named by the cached data' do + strategy.write(request, response) + cached = strategy.read(request) + + expect(JsonGadget.invocations).to be_empty + expect(cached.payload[:response_headers]['json_class']).to eq('JsonGadget') + end + end + describe 'storing responses' do shared_examples 'A strategy with serialization' do it 'writes the response object to the underlying cache' do diff --git a/spec/support/json_gadget.rb b/spec/support/json_gadget.rb new file mode 100644 index 0000000..c232495 --- /dev/null +++ b/spec/support/json_gadget.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# A class that records every attempt to build it through JSON.load's +# create_additions hook, so specs can assert cached entries never do that. +class JsonGadget + def self.invocations + @invocations ||= [] + end + + def self.json_create(attributes) + invocations << attributes + new + end +end From cc0d47f4f90e3380e840810b1a9c566d37ba4d37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?George=20Guimar=C3=A3es?= Date: Tue, 15 Sep 2026 17:15:55 -0300 Subject: [PATCH 2/3] fix: Do not share responses to authenticated requests without explicit permission A shared cache (the default) stored responses to requests carrying an Authorization header and served them to later requests from other callers whenever the origin omitted Vary and did not mark the response private. RFC 9111 section 3.5 requires a shared cache to leave such responses alone unless they carry public, must-revalidate or s-maxage. The check runs when storing, so what is never stored is never served, and again when reading, so entries written by earlier versions are treated as a miss rather than reused. Private caches (shared_cache: false) are unchanged. This changes behaviour for applications that relied on shared cache hits for authenticated responses. Origins can opt back in with Cache-Control: public or s-maxage. Release-As: 2.8.0 --- lib/faraday/http_cache.rb | 39 ++++++++++++++++++++++-- lib/faraday/http_cache/response.rb | 16 ++++++++++ spec/http_cache_spec.rb | 49 ++++++++++++++++++++++++++++++ spec/support/test_app.rb | 8 +++++ 4 files changed, 110 insertions(+), 2 deletions(-) diff --git a/lib/faraday/http_cache.rb b/lib/faraday/http_cache.rb index c0ea00a..3e4f070 100644 --- a/lib/faraday/http_cache.rb +++ b/lib/faraday/http_cache.rb @@ -196,7 +196,7 @@ def should_delete?(status, method) def process(env) entry = @strategy.read(@request) - return fetch(env) if entry.nil? + return fetch(env) if entry.nil? || !reusable?(entry) if entry.fresh? && !@request.no_cache? response = entry.to_response(env) @@ -270,7 +270,7 @@ def trace(operation) # # Returns nothing. def store(response) - if shared_cache? ? response.cacheable_in_shared_cache? : response.cacheable_in_private_cache? + if storable?(response) trace :store @strategy.write(@request, response) else @@ -278,6 +278,41 @@ def store(response) end end + # Internal: Checks if the response may be stored by this cache instance. + # A shared cache also refuses responses to requests that carried an + # 'Authorization' header unless the response explicitly allows it + # (RFC 9111 section 3.5), so what is never stored is never served to + # another caller. + # + # response - a 'Faraday::HttpCache::Response' instance. + # + # Returns true or false. + def storable?(response) + return response.cacheable_in_private_cache? unless shared_cache? + return false if authorization_bearing? && !response.shared_cache_authorized? + + response.cacheable_in_shared_cache? + end + + # Internal: Checks if a stored entry may be served for the current request. + # Entries written by earlier versions of this middleware may be responses + # to authenticated requests that a shared cache must not reuse; such an + # entry is treated as a miss and replaced. + # + # entry - a 'Faraday::HttpCache::Response' read from the strategy. + # + # Returns true or false. + def reusable?(entry) + return true unless shared_cache? && authorization_bearing? + + entry.shared_cache_authorized? + end + + # Internal: Checks if the current request carries an 'Authorization' header. + def authorization_bearing? + !@request.headers['Authorization'].nil? + end + def delete(request, response) headers = %w[Location Content-Location] headers.each do |header| diff --git a/lib/faraday/http_cache/response.rb b/lib/faraday/http_cache/response.rb index d7eeb19..2f9025d 100644 --- a/lib/faraday/http_cache/response.rb +++ b/lib/faraday/http_cache/response.rb @@ -98,6 +98,22 @@ def cacheable_in_private_cache? cacheable?(false) end + # Internal: Checks if a shared cache may reuse this response for requests + # other than the one that carried an 'Authorization' header. + # + # RFC 9111 section 3.5: a shared cache must not use a cached response to + # a request with an 'Authorization' header to satisfy any subsequent + # request unless the response carries a 'Cache-Control' directive that + # explicitly allows it. The directives with that effect are + # 'must-revalidate', 'public' and 's-maxage'. + # + # Returns true if one of those directives is present. + def shared_cache_authorized? + cache_control.public? || + cache_control.must_revalidate? || + !cache_control.shared_max_age.nil? + end + # Internal: Gets the response age in seconds. # # Returns the 'Age' header if present, or subtracts the response 'date' diff --git a/spec/http_cache_spec.rb b/spec/http_cache_spec.rb index 0021173..1a12270 100644 --- a/spec/http_cache_spec.rb +++ b/spec/http_cache_spec.rb @@ -121,6 +121,48 @@ expect(logger).to receive(:debug) { |&block| expect(block.call).to eq('HTTP Cache: [GET /private] miss, uncacheable') } client.get('private') end + + describe 'responses to requests with an "Authorization" header' do + def get_as(user, path = 'authenticated') + client.get(path) { |request| request.headers['Authorization'] = "Bearer #{user}" } + end + + it 'does not serve one caller the response cached for another' do + alice = get_as('alice') + bob = get_as('bob') + + expect(alice.body).to eq('1:Bearer alice') + expect(bob.body).to eq('2:Bearer bob') + end + + it 'logs that the response is uncacheable' do + expect(logger).to receive(:debug) { |&block| expect(block.call).to eq('HTTP Cache: [GET /authenticated] miss, uncacheable') } + get_as('alice') + end + + it 'caches responses that are explicitly marked as public' do + get_as('alice', 'authenticated-public') + bob = get_as('bob', 'authenticated-public') + + expect(bob.body).to eq('1:Bearer alice') + end + + it 'does not serve entries stored before the authorization check existed' do + store = Faraday::HttpCache::MemoryStore.new + clients = [false, true].map do |shared| + Faraday.new(url: ENV['FARADAY_SERVER']) do |stack| + stack.use Faraday::HttpCache, store: store, shared_cache: shared + stack.adapter ENV['FARADAY_ADAPTER'].to_sym + end + end + private_client, shared_client = clients + + private_client.get('authenticated') { |request| request.headers['Authorization'] = 'Bearer alice' } + bob = shared_client.get('authenticated') { |request| request.headers['Authorization'] = 'Bearer bob' } + + expect(bob.body).to eq('2:Bearer bob') + end + end end describe 'when acting as a private cache' do @@ -135,6 +177,13 @@ expect(logger).to receive(:debug) { |&block| expect(block.call).to eq('HTTP Cache: [GET /private] miss, store') } client.get('private') end + + it 'caches responses to requests with an "Authorization" header' do + client.get('authenticated') { |request| request.headers['Authorization'] = 'Bearer alice' } + bob = client.get('authenticated') { |request| request.headers['Authorization'] = 'Bearer bob' } + + expect(bob.body).to eq('1:Bearer alice') + end end it 'does not cache responses with a explicit no-store directive' do diff --git a/spec/support/test_app.rb b/spec/support/test_app.rb index 9b492fa..0d637f1 100644 --- a/spec/support/test_app.rb +++ b/spec/support/test_app.rb @@ -85,6 +85,14 @@ class TestApp < Sinatra::Base halt 405 end + get '/authenticated' do + [200, { 'Cache-Control' => 'max-age=200' }, "#{increment_counter}:#{env['HTTP_AUTHORIZATION']}"] + end + + get '/authenticated-public' do + [200, { 'Cache-Control' => 'public, max-age=200' }, "#{increment_counter}:#{env['HTTP_AUTHORIZATION']}"] + end + get '/private' do [200, { 'Cache-Control' => 'private, max-age=100' }, increment_counter] end From 43b2ea825c63ca1fc4cbf40998bc594095cdb542 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?George=20Guimar=C3=A3es?= Date: Tue, 15 Sep 2026 17:31:46 -0300 Subject: [PATCH 3/3] docs: Describe the serializer and shared cache security behaviour in the README Only recommend Marshal for fully trusted stores, since Marshal.load instantiates whatever is in the data, and document the RFC 9111 section 3.5 rule for responses to authenticated requests. --- README.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index be7a669..180a3dc 100644 --- a/README.md +++ b/README.md @@ -62,8 +62,10 @@ you might see errors like: Response could not be serialized: "\xC3" from ASCII-8BIT to UTF-8. Try using Marshal to serialize. ``` -For full unicode support, or if you expect to be dealing with images, you can use the stdlib -[Marshal][marshal] instead. Alternatively you could use another json library like `oj` or `yajl-ruby`. +For full unicode support, or if you expect to be dealing with images, you can use another json +library like `oj` or `yajl-ruby`, or the stdlib [Marshal][marshal]. Only pick Marshal when you fully +trust the cache store: `Marshal.load` will instantiate any object found in the data, while the +default `JSON` serializer parses entries into plain hashes and never instantiates classes. ```ruby client = Faraday.new do |builder| @@ -214,9 +216,11 @@ The `max-age`, `must-revalidate`, `proxy-revalidate`, `s-maxage` and ### Shared vs. non-shared caches -By default, the middleware acts as a "shared cache" per RFC 2616. This means it does not cache -responses with `Cache-Control: private`. This behavior can be changed by passing in the -`:shared_cache` configuration option: +By default, the middleware acts as a "shared cache" per RFC 9111. This means it does not cache +responses with `Cache-Control: private`, and it only stores and reuses responses to requests that +carried an `Authorization` header when the response explicitly allows it with `public`, +`must-revalidate` or `s-maxage` (RFC 9111 section 3.5). This behavior can be changed by passing in +the `:shared_cache` configuration option: ```ruby client = Faraday.new do |builder|