Merge pull request #515 from bolshakov/fix-normalize_ref

Fix caching issues in JSON::Util::URI
voxpupuli · Aug 19, 2024 · 96d8bd5 · 96d8bd5
2 parents d890318 + 2ad9e19
commit 96d8bd5
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 135 deletions.
diff --git a/lib/json-schema/util/uri.rb b/lib/json-schema/util/uri.rb
@@ -1,109 +1,132 @@
+# frozen_string_literal: true
+
 require 'addressable/uri'
 
 module JSON
   module Util
-    module URI
+    # @api private
+    class URI < Addressable::URI
       SUPPORTED_PROTOCOLS = %w(http https ftp tftp sftp ssh svn+ssh telnet nntp gopher wais ldap prospero)
 
-      def self.normalized_uri(uri, base_path = Dir.pwd)
-        @normalize_cache ||= {}
-        normalized_uri = @normalize_cache[uri]
-
-        if !normalized_uri
-          normalized_uri = parse(uri)
-          # Check for absolute path
-          if normalized_uri.relative?
-            data = normalized_uri
-            data = File.join(base_path, data) if data.path[0, 1] != '/'
-            normalized_uri = file_uri(data)
-          end
-          @normalize_cache[uri] = normalized_uri.freeze
-        end
-
-        normalized_uri
-      end
+      class << self
+        alias unescape_uri unescape
 
-      def self.absolutize_ref(ref, base)
-        ref_uri = strip_fragment(ref.dup)
-
-        return ref_uri if ref_uri.absolute?
-        return parse(base) if ref_uri.path.empty?
+        # @param uri [String, Addressable::URI]
+        # @return [Addressable::URI, nil]
+        def parse(uri)
+          super(uri)
+        rescue Addressable::URI::InvalidURIError => e
+          raise JSON::Schema::UriError, e.message
+        end
 
-        uri = strip_fragment(base.dup).join(ref_uri.path)
-        normalized_uri(uri)
-      end
+        # @param uri [String, Addressable::URI]
+        # @return [Addressable::URI, nil]
+        def file_uri(uri)
+          convert_path(parse(uri).path)
+        end
 
-      def self.normalize_ref(ref, base)
-        ref_uri = parse(ref)
-        base_uri = parse(base)
+        # @param uri [String, Addressable::URI
+        # @return [String]
+        def unescaped_path(uri)
+          parse(uri).unescaped_path
+        end
 
-        ref_uri.defer_validation do
-          if ref_uri.relative?
-            ref_uri.merge!(base_uri)
+        # Strips the fragment from the URI.
+        # @param uri [String, Addressable::URI]
+        # @return [Addressable::URI]
+        def strip_fragment(uri)
+          parse(uri).strip_fragment
+        end
 
-            # Check for absolute path
-            path, fragment = ref.to_s.split('#')
-            if path.nil? || path == ''
-              ref_uri.path = base_uri.path
-            elsif path[0, 1] == '/'
-              ref_uri.path = Pathname.new(path).cleanpath.to_s
-            else
-              ref_uri.join!(path)
-            end
+        # @param uri [String, Addressable::URI]
+        # @return [Addressable::URI]
+        def normalized_uri(uri, base_path = Dir.pwd)
+          parse(uri).normalized_uri(base_path)
+        end
 
-            ref_uri.fragment = fragment
-          end
+        # Normalizes the reference URI based on the provided base URI
+        #
+        # @param ref [String, Addressable::URI]
+        # @param base [String, Addressable::URI]
+        # @return [Addressable::URI]
+        def normalize_ref(ref, base)
+          parse(ref).normalize_ref(base)
+        end
 
-          ref_uri.fragment = '' if ref_uri.fragment.nil? || ref_uri.fragment.empty?
+        def absolutize_ref(ref, base)
+          parse(ref).absolutize_ref(base)
         end
+      end
 
-        ref_uri
+      # Unencodes any percent encoded characters within a path component.
+      #
+      # @return [String]
+      def unescaped_path
+        self.class.unescape_component(path)
       end
 
-      def self.parse(uri)
-        if uri.is_a?(Addressable::URI)
-          uri.dup
+      # Strips the fragment from the URI.
+      # @return [Addressable::URI] a new instance of URI without a fragment
+      def strip_fragment
+        if fragment.nil? || fragment.empty?
+          self
         else
-          @parse_cache ||= {}
-          parsed_uri = @parse_cache[uri]
-          if parsed_uri
-            parsed_uri.dup
-          else
-            @parse_cache[uri] = Addressable::URI.parse(uri)
-          end
+          merge(fragment: '')
         end
-      rescue Addressable::URI::InvalidURIError => e
-        raise JSON::Schema::UriError, e.message
       end
 
-      def self.strip_fragment(uri)
-        parsed_uri = parse(uri)
-        if parsed_uri.fragment.nil? || parsed_uri.fragment.empty?
-          parsed_uri
+      # Normalizes the URI based on the provided base path.
+      #
+      # @param base_path [String] the base path to use for relative URIs. Defaults to the current working directory.
+      # @return [Addressable::URI] the normalized URI or nil
+      def normalized_uri(base_path = Dir.pwd)
+        if relative?
+          if path[0, 1] == '/'
+            self.class.file_uri(self)
+          else
+            self.class.file_uri(File.join(base_path, self))
+          end
         else
-          parsed_uri.merge(fragment: '')
+          self
         end
       end
 
-      def self.file_uri(uri)
-        parsed_uri = parse(uri)
+      # @param base [Addressable::URI, String]
+      # @return [Addressable::URI]
+      def normalize_ref(base)
+        base_uri = self.class.parse(base)
+        defer_validation do
+          if relative?
+            # Check for absolute path
+            path, fragment = to_s.split('#')
+            merge!(base_uri)
 
-        Addressable::URI.convert_path(parsed_uri.path)
-      end
+            if path.nil? || path == ''
+              self.path = base_uri.path
+            elsif path[0, 1] == '/'
+              self.path = Pathname.new(path).cleanpath.to_s
+            else
+              join!(path)
+            end
 
-      def self.unescape_uri(uri)
-        Addressable::URI.unescape(uri)
-      end
+            self.fragment = fragment
+          end
 
-      def self.unescaped_path(uri)
-        parsed_uri = parse(uri)
+          self.fragment = '' if self.fragment.nil? || self.fragment.empty?
+        end
 
-        Addressable::URI.unescape(parsed_uri.path)
+        self
       end
 
-      def self.clear_cache
-        @parse_cache = {}
-        @normalize_cache = {}
+      # @param base [Addressable::URI, String]
+      # @return [Addressable::URI]
+      def absolutize_ref(base)
+        ref = strip_fragment
+        if ref.absolute?
+          ref
+        else
+          self.class.strip_fragment(base).join(ref.path).normalized_uri
+        end
       end
     end
   end

diff --git a/lib/json-schema/validator.rb b/lib/json-schema/validator.rb
@@ -300,7 +300,6 @@ def schema_reader=(reader)
 
       def clear_cache
         @@schemas = {}
-        JSON::Util::URI.clear_cache
       end
 
       def schemas

diff --git a/test/uri_util_test.rb b/test/uri_util_test.rb
@@ -1,16 +1,6 @@
 require File.expand_path('../support/test_helper', __FILE__)
 
 class UriUtilTest < Minitest::Test
-  def populate_cache_with(str, &blk)
-    cached_uri = Addressable::URI.parse(str)
-    Addressable::URI.stub(:parse, cached_uri, &blk)
-    cached_uri
-  end
-
-  def teardown
-    JSON::Util::URI.clear_cache
-  end
-
   def test_normalized_uri
     str = 'https://www.google.com/search'
     uri = Addressable::URI.new(scheme: 'https',
@@ -76,54 +66,6 @@ def test_invalid_uri_parse
     end
   end
 
-  def test_normalization_cache
-    cached_uri = populate_cache_with('www.google.com') do
-      JSON::Util::URI.normalized_uri('foo')
-    end
-
-    assert_equal(cached_uri, JSON::Util::URI.normalized_uri('foo'))
-
-    JSON::Util::URI.clear_cache
-
-    refute_equal(cached_uri, JSON::Util::URI.normalized_uri('foo'))
-  end
-
-  def test_parse_cache
-    cached_uri = populate_cache_with('www.google.com') do
-      JSON::Util::URI.parse('foo')
-    end
-
-    assert_equal(cached_uri, JSON::Util::URI.parse('foo'))
-
-    JSON::Util::URI.clear_cache
-
-    refute_equal(cached_uri, JSON::Util::URI.parse('foo'))
-  end
-
-  def test_validator_clear_cache_for_normalized_uri
-    cached_uri = populate_cache_with('www.google.com') do
-      JSON::Util::URI.normalized_uri('foo')
-    end
-
-    assert_equal(cached_uri, JSON::Util::URI.normalized_uri('foo'))
-
-    validation_errors({ 'type' => 'string' }, 'foo', clear_cache: true)
-
-    refute_equal(cached_uri, JSON::Util::URI.normalized_uri('foo'))
-  end
-
-  def test_validator_clear_cache_for_parse
-    cached_uri = populate_cache_with('www.google.com') do
-      JSON::Util::URI.parse('foo')
-    end
-
-    assert_equal(cached_uri, JSON::Util::URI.parse('foo'))
-
-    validation_errors({ 'type' => 'string' }, 'foo', clear_cache: true)
-
-    refute_equal(cached_uri, JSON::Util::URI.parse('foo'))
-  end
-
   def test_ref_fragment_path
     uri = '#some-thing'
     base = 'http://www.example.com/foo/#bar'
@@ -185,7 +127,7 @@ def test_ref_addressable_uri_with_host
     base = 'http://www.example.com/hello/#world'
 
     assert_equal Addressable::URI.parse('http://www.example.com/foo-bar.com#'), JSON::Util::URI.normalize_ref(uri, base)
-    assert_equal Addressable::URI.parse('http://www.example.com/hello/#world'), JSON::Util::URI.absolutize_ref(uri, base)
+    assert_equal Addressable::URI.parse('http://www.example.com/hello/#'), JSON::Util::URI.absolutize_ref(uri, base)
   end
 
   def test_ref_addressable_uri_with_host_and_path