Fix request cache when filtering sensitive params.

CHANGELOG.md

@@ -1,3 +1,23 @@
+## v.0.5.0 (unreleased)
+
+This release contains breaking change where `url` has been renamed to
+`sensitive_url`. A `filtered_url` method is added to make it clear that
+the URL returned is filtered according to configuration while the other one
+will contain sensitive information like key and signature.
+
+The cache key is changed so it no longer uses the URL, but a digest of the URL
+as key. You may set `config.cache_key_transform` to a block passing given url
+through if you don't want this.
+
+* Fixed an issue where read/write to cache used url with sensitive data and
+  and filtered url resulting in cache miss.
+* Instrumentation payload `url` renamed `sensitive_url`.
+* Instrumentation payload added `filtered_url`.
+* Cache key is a digest of the `sensitive_url` so we don't store in cache the
+  sensitive parts of the URL.
+* Digesting cache key is configurable with `cache_key_transform`. It's a callable
+  object expected to take the url and transform it to the key you want.
+
 ## v.0.4.0
 * When mode is `driving` and `departure_time` is set all `route` objects will contain
   `duration_in_traffic_in_seconds` and `duration_in_traffic_text`.

Gemfile

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 source 'https://rubygems.org'
 
 # Specify your gem's dependencies in google_distance_matrix.gemspec

Rakefile

@@ -1,9 +1,14 @@
-require "bundler/gem_tasks"
+# frozen_string_literal: true
 
-require "rspec/core/rake_task"
+require 'bundler/gem_tasks'
+
+require 'rubocop/rake_task'
+RuboCop::RakeTask.new
+
+require 'rspec/core/rake_task'
 RSpec::Core::RakeTask.new(:spec) do |t|
-  t.rspec_opts = ["-c", "-f progress", "-r ./spec/spec_helper.rb"]
+  t.rspec_opts = ['-c', '-f progress', '-r ./spec/spec_helper.rb']
   t.pattern = 'spec/**/*_spec.rb'
 end
 
-task default: :spec
+task default: %i[rubocop spec]

google_distance_matrix.gemspec

@@ -25,6 +25,7 @@ Gem::Specification.new do |spec|
 
   spec.add_development_dependency 'bundler'
   spec.add_development_dependency 'rspec', '~> 3.5.0'
+  spec.add_development_dependency 'rubocop', '~> 0.48'
   spec.add_development_dependency 'shoulda-matchers', '~> 3.1.1'
   spec.add_development_dependency 'webmock', '~> 3.0.1'
   spec.add_development_dependency 'rake'

lib/google_distance_matrix.rb

@@ -1,29 +1,31 @@
-require "google_distance_matrix/version"
-
-require "cgi"
-require "json"
-require "active_model"
-require "active_support/core_ext/hash"
-require "google_business_api_url_signer"
-
-require "google_distance_matrix/logger"
-require "google_distance_matrix/errors"
-require "google_distance_matrix/configuration"
-require "google_distance_matrix/url_builder"
-require "google_distance_matrix/client"
-require "google_distance_matrix/client_cache"
-require "google_distance_matrix/routes_finder"
-require "google_distance_matrix/matrix"
-require "google_distance_matrix/places"
-require "google_distance_matrix/place"
-require "google_distance_matrix/route"
-require "google_distance_matrix/polyline_encoder"
+# frozen_string_literal: true
+
+require 'google_distance_matrix/version'
+
+require 'cgi'
+require 'json'
+require 'active_model'
+require 'active_support/core_ext/hash'
+require 'google_business_api_url_signer'
+
+require 'google_distance_matrix/logger'
+require 'google_distance_matrix/errors'
+require 'google_distance_matrix/configuration'
+require 'google_distance_matrix/url_builder'
+require 'google_distance_matrix/client'
+require 'google_distance_matrix/client_cache'
+require 'google_distance_matrix/routes_finder'
+require 'google_distance_matrix/matrix'
+require 'google_distance_matrix/places'
+require 'google_distance_matrix/place'
+require 'google_distance_matrix/route'
+require 'google_distance_matrix/polyline_encoder'
 
 require 'google_distance_matrix/railtie' if defined? Rails
 
-
+# Main module for the GoogleDistanceMatrix
 module GoogleDistanceMatrix
-  extend self
+  module_function
 
   def default_configuration
     @default_configuration ||= Configuration.new
@@ -38,4 +40,4 @@ def logger
   end
 end
 
-require "google_distance_matrix/log_subscriber"
+require 'google_distance_matrix/log_subscriber'

lib/google_distance_matrix/client.rb

@@ -1,46 +1,60 @@
+# frozen_string_literal: true
+
 module GoogleDistanceMatrix
+  # HTTP client making request to Google's API
   class Client
     CLIENT_ERRORS = %w[
       INVALID_REQUEST
       MAX_ELEMENTS_EXCEEDED
       OVER_QUERY_LIMIT
       REQUEST_DENIED
       UNKNOWN_ERROR
-    ]
-
-    def get(url, options = {})
+    ].freeze
+
+    # Make a GET request to given URL
+    #
+    # @param url              The URL to Google's API we'll make a request to
+    # @param instrumentation  A hash with instrumentation payload
+    # @param options          Other options we don't care about, for example we'll capture
+    #                         `configuration` option which we are not using, but the ClientCache
+    #                         is using.
+    #
+    # @return Hash with data from parsed response body
+    def get(url, instrumentation: {}, **_options)
       uri = URI.parse url
-      instrumentation = {url: url}.merge(options[:instrumentation] || {})
 
-      response = ActiveSupport::Notifications.instrument "client_request_matrix_data.google_distance_matrix", instrumentation do
+      response = ActiveSupport::Notifications.instrument(
+        'client_request_matrix_data.google_distance_matrix', instrumentation
+      ) do
         Net::HTTP.get_response uri
       end
 
+      handle response, url
+    rescue Timeout::Error => error
+      raise ServerError, error
+    end
+
+    private
+
+    def handle(response, url) # rubocop:disable Metrics/MethodLength
       case response
       when Net::HTTPSuccess
         inspect_for_client_errors! response
       when Net::HTTPRequestURITooLong
-        fail MatrixUrlTooLong.new url, UrlBuilder::MAX_URL_SIZE, response
+        raise MatrixUrlTooLong.new url, UrlBuilder::MAX_URL_SIZE, response
       when Net::HTTPClientError
-        fail ClientError.new response
+        raise ClientError, response
       when Net::HTTPServerError
-        fail ServerError.new response
+        raise ServerError, response
       else # Handle this as a request error for now. Maybe fine tune this more later.
-        fail ServerError.new response
+        raise ServerError, response
       end
-    rescue Timeout::Error => error
-      fail ServerError.new error
     end
 
-
-    private
-
     def inspect_for_client_errors!(response)
-      status = JSON.parse(response.body).fetch "status"
+      status = JSON.parse(response.body).fetch 'status'
 
-      if CLIENT_ERRORS.include? status
-        fail ClientError.new response, status
-      end
+      raise ClientError.new response, status if CLIENT_ERRORS.include? status
 
       response
     end

lib/google_distance_matrix/client_cache.rb

@@ -1,9 +1,15 @@
+# frozen_string_literal: true
+
 module GoogleDistanceMatrix
+  # Cached client, which takes care of caching data from Google API
   class ClientCache
     attr_reader :client, :cache
 
-    def self.key(url)
-      url
+    # Returns a cache key for given URL
+    #
+    # @return String
+    def self.key(url, config)
+      config.cache_key_transform.call url
     end
 
     def initialize(client, cache)
@@ -12,7 +18,7 @@ def initialize(client, cache)
     end
 
     def get(url, options = {})
-      cache.fetch self.class.key(url) do
+      cache.fetch self.class.key(url, options.fetch(:configuration)) do
         client.get url, options
       end
     end

lib/google_distance_matrix/configuration.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'digest'
+
 module GoogleDistanceMatrix
   # Public: Configuration of matrix and it's request.
   #
@@ -26,7 +28,8 @@ class Configuration
       use_encoded_polylines: false,
       protocol: 'https',
       lat_lng_scale: 5,
-      filter_parameters_in_logged_url: %w[key signature].freeze
+      filter_parameters_in_logged_url: %w[key signature].freeze,
+      cache_key_transform: ->(url) { Digest::SHA512.new.update(url).to_s }
     }.with_indifferent_access
 
     attr_accessor(*ATTRIBUTES)
@@ -52,6 +55,10 @@ class Configuration
     # When logging we filter sensitive parameters
     attr_accessor :filter_parameters_in_logged_url
 
+    # Callable object which transform given url to key used in cache
+    # @see ClientCache
+    attr_accessor :cache_key_transform
+
     validates :mode, inclusion: { in: %w[driving walking bicycling transit] }, allow_blank: true
     validates :avoid, inclusion: { in: %w[tolls highways ferries indoor] }, allow_blank: true
     validates :units, inclusion: { in: %w[metric imperial] }, allow_blank: true

lib/google_distance_matrix/errors.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module GoogleDistanceMatrix
   # Public: Error class for lib.
   class Error < StandardError
@@ -67,15 +69,17 @@ def initialize(response, status_read_from_api_response = nil)
     end
 
     def to_s
-      "GoogleDistanceMatrix::ClientError - #{[response, status_read_from_api_response].compact.join('. ')}."
+      "GoogleDistanceMatrix::ClientError - \
+        #{[response, status_read_from_api_response].compact.join('. ')}."
     end
   end
 
   # Public: API URL was too long
   #
   # See https://developers.google.com/maps/documentation/distancematrix/#Limits, which states:
   # "Distance Matrix API URLs are restricted to 2048 characters, before URL encoding."
-  # "As some Distance Matrix API service URLs may involve many locations, be aware of this limit when constructing your URLs."
+  # "As some Distance Matrix API service URLs may involve many locations,
+  # be aware of this limit when constructing your URLs."
   #
   class MatrixUrlTooLong < ClientError
     attr_reader :url, :max_url_size
@@ -91,5 +95,4 @@ def to_s
       "Matrix API URL max size is: #{max_url_size}. Built URL was: #{url.length}. URL: '#{url}'."
     end
   end
-
 end

lib/google_distance_matrix/log_subscriber.rb

@@ -19,20 +19,10 @@ def initialize(
     end
 
     def client_request_matrix_data(event)
-      url = filter_url! event.payload[:url]
+      url = event.payload[:filtered_url]
       logger.info "(#{event.duration}ms) (elements: #{event.payload[:elements]}) GET #{url}",
                   tag: :client
     end
-
-    private
-
-    def filter_url!(url)
-      config.filter_parameters_in_logged_url.each do |param|
-        url.gsub! %r{(#{param})=.*?(&|$)}, '\1=[FILTERED]\2'
-      end
-
-      url
-    end
   end
 end
 

lib/google_distance_matrix/logger.rb

@@ -1,7 +1,10 @@
+# frozen_string_literal: true
+
 module GoogleDistanceMatrix
+  # Logger class for Google Distance Matrix
   class Logger
-    PREFIXES = %w[google_distance_matrix]
-    LEVELS = %w[fatal error warn info debug]
+    PREFIXES = %w[google_distance_matrix].freeze
+    LEVELS = %w[fatal error warn info debug].freeze
 
     attr_reader :backend
 
@@ -20,13 +23,12 @@ def initialize(backend = nil)
       end
     end
 
-
     private
 
     def tag_msg(msg, tags)
       msg_buffer = tags.map { |tag| "[#{tag}]" }
       msg_buffer << msg
-      msg_buffer.join " "
+      msg_buffer.join ' '
     end
   end
 end