Force UTF-8 encoding only if needed (#341)

Co-authored-by: Philip Ross <[email protected]>
Co-authored-by: Philip Ross <[email protected]>
Co-authored-by: Bob Aman <[email protected]>

Author: 3 people

lib/addressable/uri.rb

@@ -899,7 +899,7 @@ def normalized_scheme
         end
       end
       # All normalized values should be UTF-8
-      @normalized_scheme.force_encoding(Encoding::UTF_8) if @normalized_scheme
+      force_utf8_encoding_if_needed(@normalized_scheme)
       @normalized_scheme
     end
 
@@ -954,7 +954,7 @@ def normalized_user
         end
       end
       # All normalized values should be UTF-8
-      @normalized_user.force_encoding(Encoding::UTF_8) if @normalized_user
+      force_utf8_encoding_if_needed(@normalized_user)
       @normalized_user
     end
 
@@ -1011,9 +1011,7 @@ def normalized_password
         end
       end
       # All normalized values should be UTF-8
-      if @normalized_password
-        @normalized_password.force_encoding(Encoding::UTF_8)
-      end
+      force_utf8_encoding_if_needed(@normalized_password)
       @normalized_password
     end
 
@@ -1081,9 +1079,7 @@ def normalized_userinfo
         end
       end
       # All normalized values should be UTF-8
-      if @normalized_userinfo
-        @normalized_userinfo.force_encoding(Encoding::UTF_8)
-      end
+      force_utf8_encoding_if_needed(@normalized_userinfo)
       @normalized_userinfo
     end
 
@@ -1150,9 +1146,7 @@ def normalized_host
         end
       end
       # All normalized values should be UTF-8
-      if @normalized_host && !@normalized_host.empty?
-        @normalized_host.force_encoding(Encoding::UTF_8)
-      end
+      force_utf8_encoding_if_needed(@normalized_host)
       @normalized_host
     end
 
@@ -1270,9 +1264,7 @@ def normalized_authority
         authority
       end
       # All normalized values should be UTF-8
-      if @normalized_authority
-        @normalized_authority.force_encoding(Encoding::UTF_8)
-      end
+      force_utf8_encoding_if_needed(@normalized_authority)
       @normalized_authority
     end
 
@@ -1506,7 +1498,7 @@ def normalized_site
         site_string
       end
       # All normalized values should be UTF-8
-      @normalized_site.force_encoding(Encoding::UTF_8) if @normalized_site
+      force_utf8_encoding_if_needed(@normalized_site)
       @normalized_site
     end
 
@@ -1569,7 +1561,7 @@ def normalized_path
         result
       end
       # All normalized values should be UTF-8
-      @normalized_path.force_encoding(Encoding::UTF_8) if @normalized_path
+      force_utf8_encoding_if_needed(@normalized_path)
       @normalized_path
     end
 
@@ -1645,7 +1637,7 @@ def normalized_query(*flags)
         component == "" ? nil : component
       end
       # All normalized values should be UTF-8
-      @normalized_query.force_encoding(Encoding::UTF_8) if @normalized_query
+      force_utf8_encoding_if_needed(@normalized_query)
       @normalized_query
     end
 
@@ -1841,9 +1833,7 @@ def normalized_fragment
         component == "" ? nil : component
       end
       # All normalized values should be UTF-8
-      if @normalized_fragment
-        @normalized_fragment.force_encoding(Encoding::UTF_8)
-      end
+      force_utf8_encoding_if_needed(@normalized_fragment)
       @normalized_fragment
     end
 
@@ -2556,5 +2546,15 @@ def remove_composite_values
       remove_instance_variable(:@uri_string) if defined?(@uri_string)
       remove_instance_variable(:@hash) if defined?(@hash)
     end
+
+    ##
+    # Converts the string to be UTF-8 if it is not already UTF-8
+    #
+    # @api private
+    def force_utf8_encoding_if_needed(str)
+      if str && str.encoding != Encoding::UTF_8
+        str.force_encoding(Encoding::UTF_8)
+      end
+    end
   end
 end

spec/addressable/uri_spec.rb

@@ -998,6 +998,72 @@ def to_s
   end
 end
 
+describe Addressable::URI, "when normalized and then deeply frozen" do
+  before do
+    @uri = Addressable::URI.parse(
+      "http://user:[email protected]:8080/path?query=value#fragment"
+    ).normalize!
+
+    @uri.instance_variables.each do |var|
+      @uri.instance_variable_set(var, @uri.instance_variable_get(var).freeze)
+    end
+
+    @uri.freeze
+  end
+
+  it "#normalized_scheme should not error" do
+    expect { @uri.normalized_scheme }.not_to raise_error
+  end
+
+  it "#normalized_user should not error" do
+    expect { @uri.normalized_user }.not_to raise_error
+  end
+
+  it "#normalized_password should not error" do
+    expect { @uri.normalized_password }.not_to raise_error
+  end
+
+  it "#normalized_userinfo should not error" do
+    expect { @uri.normalized_userinfo }.not_to raise_error
+  end
+
+  it "#normalized_host should not error" do
+    expect { @uri.normalized_host }.not_to raise_error
+  end
+
+  it "#normalized_authority should not error" do
+    expect { @uri.normalized_authority }.not_to raise_error
+  end
+
+  it "#normalized_port should not error" do
+    expect { @uri.normalized_port }.not_to raise_error
+  end
+
+  it "#normalized_site should not error" do
+    expect { @uri.normalized_site }.not_to raise_error
+  end
+
+  it "#normalized_path should not error" do
+    expect { @uri.normalized_path }.not_to raise_error
+  end
+
+  it "#normalized_query should not error" do
+    expect { @uri.normalized_query }.not_to raise_error
+  end
+
+  it "#normalized_fragment should not error" do
+    expect { @uri.normalized_fragment }.not_to raise_error
+  end
+
+  it "should be frozen" do
+    expect(@uri).to be_frozen
+  end
+
+  it "should not allow destructive operations" do
+    expect { @uri.normalize! }.to raise_error(RuntimeError)
+  end
+end
+
 describe Addressable::URI, "when created from string components" do
   before do
     @uri = Addressable::URI.new(