From 8af6ac2a47dd399de856c124b5055d3cbfe24fa5 Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Thu, 30 Mar 2017 16:08:53 +1300
Subject: [PATCH] Fix a couple of tricky edge cases.

---
 lib/trenni/uri.rb       | 49 +++++++++++++++++++++++++----------------
 lib/trenni/version.rb   |  2 +-
 spec/trenni/uri_spec.rb | 13 +++++++++++
 3 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/lib/trenni/uri.rb b/lib/trenni/uri.rb
index 367f2af..f6e09d7 100644
--- a/lib/trenni/uri.rb
+++ b/lib/trenni/uri.rb
@@ -20,21 +20,32 @@
 
 module Trenni
 	class URI
-		def initialize(base, fragment, query)
-			@base = base
+		def initialize(path, query_string, fragment, parameters)
+			@path = path
+			@query_string = query_string
 			@fragment = fragment
-			@query = query
+			@parameters = parameters
 		end
 		
-		attr :base
+		# The path component of the URI, e.g. /foo/bar/index.html
+		attr :path
+		
+		# The un-parsed query string of the URI, e.g. 'x=10&y=20'
+		attr :query_string
+		
+		# A fragment identifier, the part after the '#'
 		attr :fragment
-		attr :query
+		
+		# User supplied parameters that will be appended to the query part.
+		attr :parameters
 		
 		def append(buffer)
-			buffer << escape_path(@base)
-			
-			if @query&.any?
-				buffer << query_separator << query_part
+			if @query_string
+				buffer << escape_path(@path) << '?' << query_string
+				buffer << '&' << query_parameters if @parameters
+			else
+				buffer << escape_path(@path)
+				buffer << '?' << query_parameters if @parameters
 			end
 			
 			if @fragment
@@ -52,10 +63,12 @@ def to_str
 		
 		private
 		
-		# Escapes a path string, using percent encoding, but additionally ignoring "/" and substituting spaces with "+".
+		# According to https://tools.ietf.org/html/rfc3986#section-3.3, we escape non-pchar.
+		NON_PCHAR = /([^ a-zA-Z0-9\-\.~!$&'()*+,;=:@\/]+)/.freeze
+		
 		def escape_path(path)
 			encoding = path.encoding
-			path.b.gsub(/([^ a-zA-Z0-9_.\-\/:]+)/) do |m|
+			path.b.gsub(NON_PCHAR) do |m|
 				'%' + m.unpack('H2' * m.bytesize).join('%').upcase
 			end.tr(' ', '+').force_encoding(encoding)
 		end
@@ -68,18 +81,16 @@ def escape(string)
 			end.force_encoding(encoding)
 		end
 		
-		def query_part
-			@query.map{|k,v| "#{escape(k.to_s)}=#{escape(v.to_s)}"}.join('&')
-		end
-		
-		def query_separator
-			@base.include?('?') ? '&' : '?'
+		def query_parameters
+			@parameters.map{|k,v| "#{escape(k.to_s)}=#{escape(v.to_s)}"}.join('&')
 		end
 	end
 	
-	def self.URI(path = '', query = nil)
+	# Generate a URI from a path and user parameters. The path may contain a `#fragment` or `?query=parameters`.
+	def self.URI(path = '', parameters = nil)
 		base, fragment = path.split('#', 2)
+		path, query_string = base.split('?', 2)
 		
-		URI.new(base, fragment, query)
+		URI.new(path, query_string, fragment, parameters)
 	end
 end
diff --git a/lib/trenni/version.rb b/lib/trenni/version.rb
index e84cc06..0e6840b 100644
--- a/lib/trenni/version.rb
+++ b/lib/trenni/version.rb
@@ -19,5 +19,5 @@
 # THE SOFTWARE.
 
 module Trenni
-	VERSION = "3.1.0"
+	VERSION = "3.1.1"
 end
diff --git a/spec/trenni/uri_spec.rb b/spec/trenni/uri_spec.rb
index 6ff5170..ad50f5f 100644
--- a/spec/trenni/uri_spec.rb
+++ b/spec/trenni/uri_spec.rb
@@ -52,4 +52,17 @@
 		
 		expect(tag.to_s).to be == '<img src="image.jpg?x=10"/>'
 	end
+	
+	describe Trenni::URI("foo?bar=10&baz=20", yes: 'no') do
+		it "can use existing query parameters" do
+			expect(subject.to_s).to be == "foo?bar=10&baz=20&yes=no"
+		end
+	end
+	
+	describe Trenni::URI('foo#frag') do
+		it "can use existing fragment" do
+			expect(subject.fragment).to be == "frag"
+			expect(subject.to_s).to be == 'foo#frag'
+		end
+	end
 end