Skip to content

Commit

Permalink
Hash request URIs before using them as a cache key
Browse files Browse the repository at this point in the history
Issue #132 notes that the caching middleware can generate keys
which are too long when the input is a complex URL, leading to
errors like
`Errno::ENAMETOOLONG: File name too long @ rb_sysopen`. (The
effective length limit depends on what cache driver you're using.)

At the moment, we use the normalised request URI as the cache key.
The fixes the issue by using `Digest::SHA1.hexdigest` to hash
the URLs, giving us a constant 20-byte result.

The trade-off of using a hash is that:

* there is the potential for collisions (as rare as they will
realistically be!)
* performing a hash takes time
  • Loading branch information
Tim Rogers committed Nov 26, 2018
1 parent ea159c7 commit 8922555
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
4 changes: 3 additions & 1 deletion lib/faraday_middleware/response/caching.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'faraday'
require 'forwardable'
require 'digest/sha1'
# fixes normalizing query strings:
require 'faraday_middleware/addressable_patch' if defined? ::Addressable::URI

Expand Down Expand Up @@ -66,7 +67,8 @@ def cache_key(env)
url.query = params.any? ? build_query(params) : nil
end
url.normalize!
url.request_uri

Digest::SHA1.hexdigest(url.request_uri)
end

def params_to_ignore
Expand Down
4 changes: 2 additions & 2 deletions spec/unit/caching_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
let(:options) { {:write_options => {:expires_in => 9000 } } }

it "passes on the options when writing to the cache" do
expect(@cache).to receive(:write).with("/",
expect(@cache).to receive(:write).with(Digest::SHA1.hexdigest("/"),
instance_of(Faraday::Response),
options[:write_options])
get('/')
Expand All @@ -92,7 +92,7 @@
let(:options) { {} }

it "doesn't pass a third options parameter to the cache's #write" do
expect(@cache).to receive(:write).with("/", instance_of(Faraday::Response))
expect(@cache).to receive(:write).with(Digest::SHA1.hexdigest("/"), instance_of(Faraday::Response))
get('/')
end
end
Expand Down

0 comments on commit 8922555

Please sign in to comment.