Bug 1389251 - Add esc_Spaces that may be used to force escaping of spaces r=bz,jdescottes
☠☠ backed out by 9ca74dd45440 ☠ ☠
authorValentin Gosu <valentin.gosu@gmail.com>
Tue, 12 Jun 2018 03:05:03 +0200
changeset 422620 c6baebf7b34cc0befd68d2f08658cac4668d1f32
parent 422619 6e89a11ae28ef7c50f0a91132a3ed5ce3fe61c65
child 422621 a76a3251a9d242186b5de495e4388e56390b8fc8
push id34139
push useraciure@mozilla.com
push dateFri, 15 Jun 2018 09:48:05 +0000
treeherdermozilla-central@dc997a4e045e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, jdescottes
bugs1389251
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1389251 - Add esc_Spaces that may be used to force escaping of spaces r=bz,jdescottes MozReview-Commit-ID: 7VgNlkWqrPK
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
@@ -985,17 +985,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) {
@@ -5496,17 +5496,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");
+}