From 918536d031e8bf36259edf54b3e2a2c0ac78022f Mon Sep 17 00:00:00 2001
From: Tim Hall <tim.hall.engr@gmail.com>
Date: Thu, 3 Mar 2016 19:08:53 -0500
Subject: [PATCH] Fix UrlEncode edge cases

---
 specs/Specs_WebHelpers.bas |  5 +++--
 specs/Specs_WebRequest.bas |  8 ++++----
 src/WebHelpers.bas         | 36 +++++++++++++++++++++++++++++-------
 3 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/specs/Specs_WebHelpers.bas b/specs/Specs_WebHelpers.bas
index 358c4d35..1cce46a9 100644
--- a/specs/Specs_WebHelpers.bas
+++ b/specs/Specs_WebHelpers.bas
@@ -194,9 +194,10 @@ Public Function Specs() As SpecSuite
     ' UrlEncode
     ' --------------------------------------------- '
     With Specs.It("should url-encode string (with space as plus and encode unsafe options)")
-        .Expect(WebHelpers.UrlEncode("$&+,/:;=?@", EncodeUnsafe:=False)).ToEqual "%24%26%2B%2C%2F%3A%3B%3D%3F%40"
+        .Expect(WebHelpers.UrlEncode("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890$-_.+!*'(),")).ToEqual "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890$-_.+!*'(),"
+        .Expect(WebHelpers.UrlEncode("&/:;=?@")).ToEqual "%26%2F%3A%3B%3D%3F%40"
         .Expect(WebHelpers.UrlEncode(" ""<>#%{}|\^~[]`")).ToEqual "%20%22%3C%3E%23%25%7B%7D%7C%5C%5E%7E%5B%5D%60"
-        .Expect(WebHelpers.UrlEncode("A + B")).ToEqual "A%20%2B%20B"
+        .Expect(WebHelpers.UrlEncode("A + B")).ToEqual "A%20+%20B"
         .Expect(WebHelpers.UrlEncode("A + B", SpaceAsPlus:=True)).ToEqual "A+%2B+B"
     End With
     
diff --git a/specs/Specs_WebRequest.bas b/specs/Specs_WebRequest.bas
index 0e53a36a..083f3c93 100644
--- a/specs/Specs_WebRequest.bas
+++ b/specs/Specs_WebRequest.bas
@@ -228,9 +228,9 @@ Public Function Specs() As SpecSuite
         Set Request = New WebRequest
         
         Request.Resource = "{segment}"
-        Request.AddUrlSegment "segment", "$&+,/:;=?@"
+        Request.AddUrlSegment "segment", "&/:;=?@"
         
-        .Expect(Request.FormattedResource).ToEqual "%24%26%2B%2C%2F%3A%3B%3D%3F%40"
+        .Expect(Request.FormattedResource).ToEqual "%26%2F%3A%3B%3D%3F%40"
     End With
     
     With Specs.It("FormattedResource should include querystring parameters")
@@ -266,9 +266,9 @@ Public Function Specs() As SpecSuite
     With Specs.It("FormattedResource should URL encode querystring")
         Set Request = New WebRequest
     
-        Request.AddQuerystringParam "A B", "$&+,/:;=?@"
+        Request.AddQuerystringParam "A B", "&/:;=?@"
         
-        .Expect(Request.FormattedResource).ToEqual "?A+B=%24%26%2B%2C%2F%3A%3B%3D%3F%40"
+        .Expect(Request.FormattedResource).ToEqual "?A+B=%26%2F%3A%3B%3D%3F%40"
     End With
     
     ' UserAgent
diff --git a/src/WebHelpers.bas b/src/WebHelpers.bas
index 666b240e..84bdcf5f 100644
--- a/src/WebHelpers.bas
+++ b/src/WebHelpers.bas
@@ -812,7 +812,14 @@ End Function
 
 ''
 ' Encode string for URLs
-' Reference: http://www.blooberry.com/indexdot/html/topics/urlencoding.htm
+' Reference:
+' - http://www.blooberry.com/indexdot/html/topics/urlencoding.htm
+' - https://www.ietf.org/rfc/rfc1738.txt
+'
+' From RFC 1738:
+' > Thus, only alphanumerics, the special characters "$-_.+!*'(),", and
+' reserved characters used for their reserved purposes may be used
+' unencoded within a URL.
 '
 ' @method UrlEncode
 ' @param {Variant} Text Text to encode
@@ -850,19 +857,34 @@ Public Function UrlEncode(Text As Variant, Optional SpaceAsPlus As Boolean = Fal
             web_CharCode = VBA.Asc(web_Char)
 
             Select Case web_CharCode
-                Case 97 To 122, 65 To 90, 48 To 57, 45, 46, 95, 126
+                Case 33, 36, 39, 40, 41, 42, 44, 45, 46, 48 To 57, 65 To 90, 95, 97 To 122
+                    ' Unencoded:
+                    ' alphanumeric - 48-57, 65-90, 97-122
+                    ' $-_.!*'(), - 33, 36, 39, 40, 41, 42, 43, 44, 45, 46, 95
                     web_Result(web_i) = web_Char
-                Case 32
-                    web_Result(web_i) = web_Space
-                Case 0 To 15
-                    web_Result(web_i) = "%0" & VBA.Hex(web_CharCode)
                 Case 34, 35, 37, 60, 62, 91 To 94, 96, 123 To 126
-                    ' Unsafe characters
+                    ' Unsafe characters: <>"#%{}|\^~[]`
+                    If EncodeUnsafe Then
+                        web_Result(web_i) = "%" & VBA.Hex(web_CharCode)
+                    Else
+                        web_Result(web_i) = web_Char
+                    End If
+                Case 32
                     If EncodeUnsafe Then
+                        web_Result(web_i) = web_Space
+                    Else
+                        web_Result(web_i) = web_Char
+                    End If
+                Case 43
+                    ' + is considered safe special character
+                    ' but in space-as-plus cases, it's encoded to differentiate with space
+                    If EncodeUnsafe And SpaceAsPlus Then
                         web_Result(web_i) = "%" & VBA.Hex(web_CharCode)
                     Else
                         web_Result(web_i) = web_Char
                     End If
+                Case 0 To 15
+                    web_Result(web_i) = "%0" & VBA.Hex(web_CharCode)
                 Case Else
                     web_Result(web_i) = "%" & VBA.Hex(web_CharCode)
             End Select