Bug 1389251 - Add esc_Spaces that may be used to force escaping of spaces r=bz draft
authorValentin Gosu <valentin.gosu@gmail.com>
Mon, 18 Jun 2018 15:06:25 +0200
changeset 808129 7ff705a16984bca7bd318864ab2b0b3a89455805
parent 808128 2da1b5f305ca2abcce2f9988cd6a5cbc12635c61
child 808130 dbac446cbf4a94c2f6c927ef2fa68444751c03a1
push id113284
push uservalentin.gosu@gmail.com
push dateMon, 18 Jun 2018 13:48:27 +0000
reviewersbz
bugs1389251
milestone62.0a1
Bug 1389251 - Add esc_Spaces that may be used to force escaping of spaces r=bz MozReview-Commit-ID: 4tahH3IOKW
dom/media/webspeech/synth/speechd/SpeechDispatcherService.cpp
netwerk/base/nsSimpleURI.cpp
netwerk/protocol/http/InterceptedHttpChannel.cpp
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/protocol/http/nsHttpDigestAuth.cpp
netwerk/test/unit/test_URIs.js
xpcom/io/nsEscape.cpp
xpcom/io/nsEscape.h
xpcom/tests/gtest/TestEscape.cpp
--- a/dom/media/webspeech/synth/speechd/SpeechDispatcherService.cpp
+++ b/dom/media/webspeech/synth/speechd/SpeechDispatcherService.cpp
@@ -388,17 +388,17 @@ SpeechDispatcherService::Setup()
   spd_set_notification_on(mSpeechdClient, SPD_CANCEL);
 
   if (list != NULL) {
     for (int i = 0; list[i]; i++) {
       nsAutoString uri;
 
       uri.AssignLiteral(URI_PREFIX);
       nsAutoCString name;
-      NS_EscapeURL(list[i]->name, -1, esc_OnlyNonASCII | esc_AlwaysCopy, name);
+      NS_EscapeURL(list[i]->name, -1, esc_OnlyNonASCII | esc_Spaces | esc_AlwaysCopy, name);
       uri.Append(NS_ConvertUTF8toUTF16(name));;
       uri.AppendLiteral("?");
 
       nsAutoCString lang(list[i]->language);
 
       if (strcmp(list[i]->variant, "none") != 0) {
         // In speech dispatcher, the variant will usually be the locale subtag
         // with another, non-standard suptag after it. We keep the first one
--- a/netwerk/base/nsSimpleURI.cpp
+++ b/netwerk/base/nsSimpleURI.cpp
@@ -507,17 +507,17 @@ nsSimpleURI::GetRef(nsACString &result)
 }
 
 // NOTE: SetRef("") removes our ref, whereas SetRef("#") sets it to the empty
 // string (and will result in .spec and .path having a terminal #).
 nsresult
 nsSimpleURI::SetRef(const nsACString &aRef)
 {
     nsAutoCString ref;
-    nsresult rv = NS_EscapeURL(aRef, esc_OnlyNonASCII, ref, fallible);
+    nsresult rv = NS_EscapeURL(aRef, esc_OnlyNonASCII | esc_Spaces, ref, fallible);
     if (NS_FAILED(rv)) {
         return rv;
     }
 
     if (ref.IsEmpty()) {
         // Empty string means to remove ref completely.
         mIsRefValid = false;
         mRef.Truncate(); // invariant: mRef should be empty when it's not valid
--- a/netwerk/protocol/http/InterceptedHttpChannel.cpp
+++ b/netwerk/protocol/http/InterceptedHttpChannel.cpp
@@ -180,17 +180,17 @@ InterceptedHttpChannel::FollowSyntheticR
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsAutoCString location;
   rv = mResponseHead->GetHeader(nsHttp::Location, location);
   NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
 
   // make sure non-ASCII characters in the location header are escaped.
   nsAutoCString locationBuf;
-  if (NS_EscapeURL(location.get(), -1, esc_OnlyNonASCII, locationBuf)) {
+  if (NS_EscapeURL(location.get(), -1, esc_OnlyNonASCII | esc_Spaces, locationBuf)) {
     location = locationBuf;
   }
 
   nsCOMPtr<nsIURI> redirectURI;
   rv = ioService->NewURI(nsDependentCString(location.get()),
                          nullptr,
                          mURI,
                          getter_AddRefs(redirectURI));
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -987,17 +987,17 @@ nsHttpChannel::SetupTransaction()
 
     // This is the normal e2e H1 path syntax "/index.html"
     rv = mURI->GetPathQueryRef(path);
     if (NS_FAILED(rv)) {
         return rv;
     }
 
     // path may contain UTF-8 characters, so ensure that they're escaped.
-    if (NS_EscapeURL(path.get(), path.Length(), esc_OnlyNonASCII, buf)) {
+    if (NS_EscapeURL(path.get(), path.Length(), esc_OnlyNonASCII | esc_Spaces, buf)) {
         requestURI = &buf;
     } else {
         requestURI = &path;
     }
 
     // trim off the #ref portion if any...
     int32_t ref1 = requestURI->FindChar('#');
     if (ref1 != kNotFound) {
@@ -5498,17 +5498,17 @@ nsHttpChannel::AsyncProcessRedirection(u
 
     // if a location header was not given, then we can't perform the redirect,
     // so just carry on as though this were a normal response.
     if (NS_FAILED(mResponseHead->GetHeader(nsHttp::Location, location)))
         return NS_ERROR_FAILURE;
 
     // make sure non-ASCII characters in the location header are escaped.
     nsAutoCString locationBuf;
-    if (NS_EscapeURL(location.get(), -1, esc_OnlyNonASCII, locationBuf))
+    if (NS_EscapeURL(location.get(), -1, esc_OnlyNonASCII | esc_Spaces, locationBuf))
         location = locationBuf;
 
     mRedirectType = redirectType;
 
     LOG(("redirecting to: %s [redirection-limit=%u]\n",
         location.get(), uint32_t(mRedirectionLimit)));
 
     nsresult rv = CreateNewURI(location.get(), getter_AddRefs(mRedirectURI));
--- a/netwerk/protocol/http/nsHttpDigestAuth.cpp
+++ b/netwerk/protocol/http/nsHttpDigestAuth.cpp
@@ -107,17 +107,17 @@ nsHttpDigestAuth::GetMethodAndPath(nsIHt
           //
           // make sure we escape any UTF-8 characters in the URI path.  the
           // digest auth uri attribute needs to match the request-URI.
           //
           // XXX we should really ask the HTTP channel for this string
           // instead of regenerating it here.
           //
           nsAutoCString buf;
-          rv = NS_EscapeURL(path, esc_OnlyNonASCII, buf, mozilla::fallible);
+          rv = NS_EscapeURL(path, esc_OnlyNonASCII | esc_Spaces, buf, mozilla::fallible);
           if (NS_SUCCEEDED(rv)) {
             path = buf;
           }
         }
       }
     }
   }
   return rv;
--- a/netwerk/test/unit/test_URIs.js
+++ b/netwerk/test/unit/test_URIs.js
@@ -589,21 +589,36 @@ function check_nested_mutations()
   uri1 = gIoService.newURI("view-source:about:blank?query#ref");
   uri2 = gIoService.newURI("view-source:about:blank");
   uri3 = uri1.mutate().setPathQueryRef("blank").finalize();
   do_check_uri_eq(uri3, uri2);
   uri3 = uri2.mutate().setPathQueryRef("blank?query#ref").finalize();
   do_check_uri_eq(uri3, uri1);
 }
 
+function check_space_escaping()
+{
+  let uri = gIoService.newURI("data:text/plain,hello%20world#space hash");
+  Assert.equal(uri.spec, "data:text/plain,hello%20world#space%20hash");
+  uri = gIoService.newURI("data:text/plain,hello%20world#space%20hash");
+  Assert.equal(uri.spec, "data:text/plain,hello%20world#space%20hash");
+  uri = gIoService.newURI("data:text/plain,hello world#space%20hash");
+  Assert.equal(uri.spec, "data:text/plain,hello world#space%20hash");
+  uri = gIoService.newURI("data:text/plain,hello world#space hash");
+  Assert.equal(uri.spec, "data:text/plain,hello world#space%20hash");
+  uri = gIoService.newURI("http://example.com/test path#test path");
+  uri = gIoService.newURI("http://example.com/test%20path#test%20path");
+}
+
 // TEST MAIN FUNCTION
 // ------------------
 function run_test()
 {
   check_nested_mutations();
+  check_space_escaping();
 
   // UTF-8 check - From bug 622981
   // ASCII
   let base = gIoService.newURI("http://example.org/xenia?");
   let resolved = gIoService.newURI("?x", null, base);
   let expected = gIoService.newURI("http://example.org/xenia?x");
   do_info("Bug 662981: ACSII - comparing " + resolved.spec + " and " + expected.spec);
   Assert.ok(resolved.equals(expected));
--- a/xpcom/io/nsEscape.cpp
+++ b/xpcom/io/nsEscape.cpp
@@ -314,16 +314,17 @@ T_EscapeURL(const typename T::char_type*
     return NS_ERROR_INVALID_ARG;
   }
 
   bool forced = !!(aFlags & esc_Forced);
   bool ignoreNonAscii = !!(aFlags & esc_OnlyASCII);
   bool ignoreAscii = !!(aFlags & esc_OnlyNonASCII);
   bool writing = !!(aFlags & esc_AlwaysCopy);
   bool colon = !!(aFlags & esc_Colon);
+  bool spaces = !!(aFlags & esc_Spaces);
 
   auto src = reinterpret_cast<const unsigned_char_type*>(aPart);
 
   typename T::char_type tempBuffer[100];
   unsigned int tempBufferPos = 0;
 
   bool previousIsNonASCII = false;
   for (size_t i = 0; i < aPartLen; ++i) {
@@ -356,16 +357,17 @@ T_EscapeURL(const typename T::char_type*
     // And, we should escape the '|' character when it occurs after any
     // non-ASCII character as it may be aPart of a multi-byte character.
     //
     // 0x20..0x7e are the valid ASCII characters.
     if ((dontNeedEscape(c, aFlags) || (c == HEX_ESCAPE && !forced)
          || (c > 0x7f && ignoreNonAscii)
          || (c >= 0x20 && c < 0x7f && ignoreAscii))
         && !(c == ':' && colon)
+        && !(c == ' ' && spaces)
         && !(previousIsNonASCII && c == '|' && !ignoreNonAscii)) {
       if (writing) {
         tempBuffer[tempBufferPos++] = c;
       }
     } else { /* do the escape magic */
       if (!writing) {
         if (!aResult.Append(aPart, i, mozilla::fallible)) {
           return NS_ERROR_OUT_OF_MEMORY;
--- a/xpcom/io/nsEscape.h
+++ b/xpcom/io/nsEscape.h
@@ -88,17 +88,18 @@ enum EscapeMask {
   esc_Minimal        = esc_Scheme | esc_Username | esc_Password | esc_Host | esc_FilePath | esc_Param | esc_Query | esc_Ref,
   esc_Forced         = 1u << 10, /* forces escaping of existing escape sequences */
   esc_OnlyASCII      = 1u << 11, /* causes non-ascii octets to be skipped */
   esc_OnlyNonASCII   = 1u << 12, /* causes _graphic_ ascii octets (0x20-0x7E)
                                     * to be skipped when escaping. causes all
                                     * ascii octets (<= 0x7F) to be skipped when unescaping */
   esc_AlwaysCopy     = 1u << 13, /* copy input to result buf even if escaping is unnecessary */
   esc_Colon          = 1u << 14, /* forces escape of colon */
-  esc_SkipControl    = 1u << 15  /* skips C0 and DEL from unescaping */
+  esc_SkipControl    = 1u << 15, /* skips C0 and DEL from unescaping */
+  esc_Spaces         = 1u << 16  /* forces escape of spaces */
 };
 
 /**
  * NS_EscapeURL
  *
  * Escapes invalid char's in an URL segment.  Has no side-effect if the URL
  * segment is already escaped, unless aFlags has the esc_Forced bit in which
  * case % will also be escaped.  Iff some part of aStr is escaped is the
--- a/xpcom/tests/gtest/TestEscape.cpp
+++ b/xpcom/tests/gtest/TestEscape.cpp
@@ -128,8 +128,24 @@ TEST(Escape, nsAppendEscapedHTML)
   nsCString dst;
   for (size_t i = 0; i < ArrayLength(srcs); i++) {
     nsCString src(srcs[i]);
     nsAppendEscapedHTML(src, dst);
     ASSERT_TRUE(dst.Equals(dsts2[i]));
   }
 }
 
+TEST(Escape, EscapeSpaces)
+{
+  // Tests the fallible version of NS_EscapeURL works as expected when no
+  // escaping is necessary.
+  nsCString toEscape("data:\x0D\x0A spa ces\xC4\x9F");
+  nsCString escaped;
+  nsresult rv = NS_EscapeURL(toEscape, esc_OnlyNonASCII, escaped, fallible);
+  EXPECT_EQ(rv, NS_OK);
+  // Only non-ASCII and C0
+  EXPECT_STREQ(escaped.BeginReading(), "data:%0D%0A spa ces%C4%9F");
+
+  escaped.Truncate();
+  rv = NS_EscapeURL(toEscape, esc_OnlyNonASCII | esc_Spaces, escaped, fallible);
+  EXPECT_EQ(rv, NS_OK);
+  EXPECT_STREQ(escaped.BeginReading(), "data:%0D%0A%20spa%20ces%C4%9F");
+}