Bug 669675 - Use Tokenizer in ParseRealm r=necko-reviewers,dragana
☠☠ backed out by a9431e068bcd ☠ ☠
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 06 May 2021 13:17:23 +0000
changeset 578743 42561f42313d350602e9c185848c1c1ccbd3a78c
parent 578742 2aee05c2d6f3250a755dfb1dd018808a4d7c3ed4
child 578744 014cd94a623c5e84237cf28bdeede22d560e76e2
push id142679
push uservalentin.gosu@gmail.com
push dateThu, 06 May 2021 13:44:58 +0000
treeherderautoland@42561f42313d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnecko-reviewers, dragana
bugs669675
milestone90.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 669675 - Use Tokenizer in ParseRealm r=necko-reviewers,dragana We also import the testcases from http://test.greenbytes.de/tech/tc/httpauth/ as unit tests. This patch adds a network.auth.use_new_parse_realm pref in case this change causes any regressions. Depends on D112605 Differential Revision: https://phabricator.services.mozilla.com/D112594
modules/libpref/init/StaticPrefList.yaml
netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
netwerk/test/unit/test_authentication.js
--- a/modules/libpref/init/StaticPrefList.yaml
+++ b/modules/libpref/init/StaticPrefList.yaml
@@ -8731,16 +8731,22 @@
 
 # Whether to show anti-spoof confirmation prompts when navigating to a url
 # with userinfo
 - name: network.auth.confirmAuth.enabled
   type: bool
   value: true
   mirror: always
 
+# Whether to use new implementation of ParseReasm
+- name: network.auth.use_new_parse_realm
+  type: RelaxedAtomicBool
+  value: true
+  mirror: always
+
 # See the full list of values in nsICookieService.idl.
 - name: network.cookie.cookieBehavior
   type: RelaxedAtomicInt32
   value: 0 # accept all cookies
   mirror: always
 
 # See the full list of values in nsICookieService.idl.
 - name: network.cookie.rejectForeignWithExceptions.enabled
--- a/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
+++ b/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
@@ -1104,18 +1104,17 @@ void nsHttpChannelAuthProvider::GetIdent
     if (authFlags & nsIHttpAuthenticator::IDENTITY_INCLUDES_DOMAIN) {
       ParseUserDomain(userBuf, user, domain);
     }
 
     ident = nsHttpAuthIdentity(domain, user, passBuf);
   }
 }
 
-void nsHttpChannelAuthProvider::ParseRealm(const nsACString& aChallenge,
-                                           nsACString& realm) {
+static void OldParseRealm(const nsACString& aChallenge, nsACString& realm) {
   //
   // From RFC2617 section 1.2, the realm value is defined as such:
   //
   //    realm       = "realm" "=" realm-value
   //    realm-value = quoted-string
   //
   // but, we'll accept anything after the the "=" up to the first space, or
   // end-of-line, if the string is not quoted.
@@ -1135,30 +1134,128 @@ void nsHttpChannelAuthProvider::ParseRea
 
     const char* end;
     if (has_quote) {
       end = p;
       while (*end) {
         if (*end == '\\') {
           // escaped character, store that one instead if not zero
           if (!*++end) break;
-        } else if (*end == '\"')
+        } else if (*end == '\"') {
           // end of string
           break;
+        }
 
         realm.Append(*end);
         ++end;
       }
     } else {
       // realm given without quotes
       end = strchr(p, ' ');
-      if (end)
+      if (end) {
         realm.Assign(p, end - p);
-      else
+      } else {
         realm.Assign(p);
+      }
+    }
+  }
+}
+
+void nsHttpChannelAuthProvider::ParseRealm(const nsACString& aChallenge,
+                                           nsACString& realm) {
+  //
+  // From RFC2617 section 1.2, the realm value is defined as such:
+  //
+  //    realm       = "realm" "=" realm-value
+  //    realm-value = quoted-string
+  //
+  // but, we'll accept anything after the the "=" up to the first space, or
+  // end-of-line, if the string is not quoted.
+  //
+
+  if (!StaticPrefs::network_auth_use_new_parse_realm()) {
+    OldParseRealm(aChallenge, realm);
+    return;
+  }
+
+  Tokenizer t(aChallenge);
+
+  // The challenge begins with the authType.
+  // If we can't find that something has probably gone wrong.
+  t.SkipWhites();
+  nsDependentCSubstring authType;
+  if (!t.ReadWord(authType)) {
+    return;
+  }
+
+  // Will return true if the tokenizer advanced the cursor - false otherwise.
+  auto readParam = [&](nsDependentCSubstring& key, nsAutoCString& value) {
+    key.Rebind(EmptyCString(), 0);
+    value.Truncate();
+
+    t.SkipWhites();
+    if (!t.ReadWord(key)) {
+      return false;
+    }
+    t.SkipWhites();
+    if (!t.CheckChar('=')) {
+      return true;
+    }
+    t.SkipWhites();
+
+    Tokenizer::Token token1;
+
+    t.Record();
+    if (!t.Next(token1)) {
+      return true;
+    }
+    nsDependentCSubstring sub;
+    bool hasQuote = false;
+    if (token1.Equals(Tokenizer::Token::Char('"'))) {
+      hasQuote = true;
+    } else {
+      t.Claim(sub, Tokenizer::ClaimInclusion::INCLUDE_LAST);
+      value.Append(sub);
+    }
+    t.Record();
+    Tokenizer::Token token2;
+    while (t.Next(token2)) {
+      if (hasQuote && token2.Equals(Tokenizer::Token::Char('"')) &&
+          !token1.Equals(Tokenizer::Token::Char('\\'))) {
+        break;
+      }
+      if (!hasQuote && (token2.Type() == Tokenizer::TokenType::TOKEN_WS ||
+                        token2.Type() == Tokenizer::TokenType::TOKEN_EOL)) {
+        break;
+      }
+
+      t.Claim(sub, Tokenizer::ClaimInclusion::INCLUDE_LAST);
+      if (!sub.Equals(R"(\)")) {
+        value.Append(sub);
+      }
+      t.Record();
+      token1 = token2;
+    }
+    return true;
+  };
+
+  while (!t.CheckEOF()) {
+    nsDependentCSubstring key;
+    nsAutoCString value;
+    // If we couldn't read anything, and the input isn't followed by a ,
+    // then we exit.
+    if (!readParam(key, value) && !t.Check(Tokenizer::Token::Char(','))) {
+      break;
+    }
+    // When we find the first instance of realm we exit.
+    // Theoretically there should be only one instance and we should fail
+    // if there are more, but we're trying to preserve existing behaviour.
+    if (key.Equals("realm"_ns, nsCaseInsensitiveCStringComparator)) {
+      realm = value;
+      break;
     }
   }
 }
 
 class nsHTTPAuthInformation : public nsAuthInformationHolder {
  public:
   nsHTTPAuthInformation(uint32_t aFlags, const nsString& aRealm,
                         const nsACString& aAuthType)
--- a/netwerk/test/unit/test_authentication.js
+++ b/netwerk/test/unit/test_authentication.js
@@ -213,34 +213,36 @@ Requestor.prototype = {
 
     throw Components.Exception("", Cr.NS_ERROR_NO_INTERFACE);
   },
 
   prompt1: null,
   prompt2: null,
 };
 
-function RealmTestRequestor() {}
+function RealmTestRequestor(expected) {
+  this.expectedRealm = expected;
+}
 
 RealmTestRequestor.prototype = {
   QueryInterface: ChromeUtils.generateQI([
     "nsIInterfaceRequestor",
     "nsIAuthPrompt2",
   ]),
 
   getInterface: function realmtest_interface(iid) {
     if (iid.equals(Ci.nsIAuthPrompt2)) {
       return this;
     }
 
     throw Components.Exception("", Cr.NS_ERROR_NO_INTERFACE);
   },
 
   promptAuth: function realmtest_checkAuth(channel, level, authInfo) {
-    Assert.equal(authInfo.realm, '"foo_bar');
+    Assert.equal(authInfo.realm, this.expectedRealm);
 
     return false;
   },
 
   asyncPromptAuth: function realmtest_async(chan, cb, ctx, lvl, info) {
     throw Components.Exception("", Cr.NS_ERROR_NOT_IMPLEMENTED);
   },
 };
@@ -414,17 +416,17 @@ add_task(async function test_ntlm() {
   chan.notificationCallbacks = new Requestor(FLAG_RETURN_FALSE, 2);
   listener.expectedCode = 401; // Unauthorized
   await openAndListen(chan);
 });
 
 add_task(async function test_basicrealm() {
   var chan = makeChan(URL + "/auth/realm", URL);
 
-  chan.notificationCallbacks = new RealmTestRequestor();
+  chan.notificationCallbacks = new RealmTestRequestor('"foo_bar');
   listener.expectedCode = 401; // Unauthorized
   await openAndListen(chan);
 });
 
 add_task(async function test_nonascii() {
   var chan = makeChan(URL + "/auth/non_ascii", URL);
 
   chan.notificationCallbacks = new Requestor(FLAG_NON_ASCII_USER_PASSWORD, 2);
@@ -901,8 +903,285 @@ add_task(async function test_large_realm
 });
 
 add_task(async function test_large_domain() {
   var chan = makeChan(URL + "/largeDomain", URL);
 
   listener.expectedCode = 401; // Unauthorized
   await openAndListen(chan);
 });
+
+async function add_parse_realm_testcase(testcase) {
+  httpserv.registerPathHandler("/parse_realm", (metadata, response) => {
+    response.setStatusLine(metadata.httpVersion, 401, "Unauthorized");
+    response.setHeader("WWW-Authenticate", testcase.input, false);
+
+    let body = "failed";
+    response.bodyOutputStream.write(body, body.length);
+  });
+
+  let chan = makeChan(URL + "/parse_realm", URL);
+  chan.notificationCallbacks = new RealmTestRequestor(testcase.realm);
+
+  listener.expectedCode = 401;
+  await openAndListen(chan);
+}
+
+add_task(async function simplebasic() {
+  await add_parse_realm_testcase({
+    input: `Basic realm="foo"`,
+    scheme: `Basic`,
+    realm: `foo`,
+  });
+});
+
+add_task(async function simplebasiclf() {
+  await add_parse_realm_testcase({
+    input: `Basic\r\n realm="foo"`,
+    scheme: `Basic`,
+    realm: `foo`,
+  });
+});
+
+add_task(async function simplebasicucase() {
+  await add_parse_realm_testcase({
+    input: `BASIC REALM="foo"`,
+    scheme: `Basic`,
+    realm: `foo`,
+  });
+});
+
+add_task(async function simplebasictok() {
+  await add_parse_realm_testcase({
+    input: `Basic realm=foo`,
+    scheme: `Basic`,
+    realm: `foo`,
+  });
+});
+
+add_task(async function simplebasictokbs() {
+  await add_parse_realm_testcase({
+    input: `Basic realm=\\f\\o\\o`,
+    scheme: `Basic`,
+    realm: `\\foo`,
+  });
+});
+
+add_task(async function simplebasicsq() {
+  await add_parse_realm_testcase({
+    input: `Basic realm='foo'`,
+    scheme: `Basic`,
+    realm: `'foo'`,
+  });
+});
+
+add_task(async function simplebasicpct() {
+  await add_parse_realm_testcase({
+    input: `Basic realm="foo%20bar"`,
+    scheme: `Basic`,
+    realm: `foo%20bar`,
+  });
+});
+
+add_task(async function simplebasiccomma() {
+  await add_parse_realm_testcase({
+    input: `Basic , realm="foo"`,
+    scheme: `Basic`,
+    realm: `foo`,
+  });
+});
+
+add_task(async function simplebasiccomma2() {
+  await add_parse_realm_testcase({
+    input: `Basic, realm="foo"`,
+    scheme: `Basic`,
+    realm: ``,
+  });
+});
+
+add_task(async function simplebasicnorealm() {
+  await add_parse_realm_testcase({
+    input: `Basic`,
+    scheme: `Basic`,
+    realm: ``,
+  });
+});
+
+add_task(async function simplebasic2realms() {
+  await add_parse_realm_testcase({
+    input: `Basic realm="foo", realm="bar"`,
+    scheme: `Basic`,
+    realm: `foo`,
+  });
+});
+
+add_task(async function simplebasicwsrealm() {
+  await add_parse_realm_testcase({
+    input: `Basic realm = "foo"`,
+    scheme: `Basic`,
+    realm: `foo`,
+  });
+});
+
+add_task(async function simplebasicrealmsqc() {
+  await add_parse_realm_testcase({
+    input: `Basic realm="\\f\\o\\o"`,
+    scheme: `Basic`,
+    realm: `foo`,
+  });
+});
+
+add_task(async function simplebasicrealmsqc2() {
+  await add_parse_realm_testcase({
+    input: `Basic realm="\\"foo\\""`,
+    scheme: `Basic`,
+    realm: `"foo"`,
+  });
+});
+
+add_task(async function simplebasicnewparam1() {
+  await add_parse_realm_testcase({
+    input: `Basic realm="foo", bar="xyz",, a=b,,,c=d`,
+    scheme: `Basic`,
+    realm: `foo`,
+  });
+});
+
+add_task(async function simplebasicnewparam2() {
+  await add_parse_realm_testcase({
+    input: `Basic bar="xyz", realm="foo"`,
+    scheme: `Basic`,
+    realm: `foo`,
+  });
+});
+
+add_task(async function simplebasicrealmiso88591() {
+  await add_parse_realm_testcase({
+    input: `Basic realm="foo-ä"`,
+    scheme: `Basic`,
+    realm: `foo-ä`,
+  });
+});
+
+add_task(async function simplebasicrealmutf8() {
+  await add_parse_realm_testcase({
+    input: `Basic realm="foo-ä"`,
+    scheme: `Basic`,
+    realm: `foo-ä`,
+  });
+});
+
+add_task(async function simplebasicrealmrfc2047() {
+  await add_parse_realm_testcase({
+    input: `Basic realm="=?ISO-8859-1?Q?foo-=E4?="`,
+    scheme: `Basic`,
+    realm: `=?ISO-8859-1?Q?foo-=E4?=`,
+  });
+});
+
+add_task(async function multibasicunknown() {
+  await add_parse_realm_testcase({
+    input: `Basic realm="basic", Newauth realm="newauth"`,
+    scheme: `Basic`,
+    realm: `basic`,
+  });
+});
+
+add_task(async function multibasicunknownnoparam() {
+  await add_parse_realm_testcase({
+    input: `Basic realm="basic", Newauth`,
+    scheme: `Basic`,
+    realm: `basic`,
+  });
+});
+
+add_task(async function multibasicunknown2() {
+  await add_parse_realm_testcase({
+    input: `Newauth realm="newauth", Basic realm="basic"`,
+    scheme: `Basic`,
+    realm: `basic`,
+  });
+});
+
+add_task(async function multibasicunknown2np() {
+  await add_parse_realm_testcase({
+    input: `Newauth, Basic realm="basic"`,
+    scheme: `Basic`,
+    realm: `basic`,
+  });
+});
+
+add_task(async function multibasicunknown2mf() {
+  httpserv.registerPathHandler("/parse_realm", (metadata, response) => {
+    response.setStatusLine(metadata.httpVersion, 401, "Unauthorized");
+    response.setHeader("WWW-Authenticate", `Newauth realm="newauth"`, false);
+    response.setHeader("WWW-Authenticate", `Basic realm="basic"`, false);
+
+    let body = "failed";
+    response.bodyOutputStream.write(body, body.length);
+  });
+
+  let chan = makeChan(URL + "/parse_realm", URL);
+  chan.notificationCallbacks = new RealmTestRequestor("basic");
+
+  listener.expectedCode = 401;
+  await openAndListen(chan);
+});
+
+add_task(async function multibasicempty() {
+  await add_parse_realm_testcase({
+    input: `,Basic realm="basic"`,
+    scheme: `Basic`,
+    realm: `basic`,
+  });
+});
+
+add_task(async function multibasicqs() {
+  await add_parse_realm_testcase({
+    input: `Newauth realm="apps", type=1, title="Login to \"apps\"", Basic realm="simple"`,
+    scheme: `Basic`,
+    realm: `simple`,
+  });
+});
+
+add_task(async function multidisgscheme() {
+  await add_parse_realm_testcase({
+    input: `Newauth realm="Newauth Realm", basic=foo, Basic realm="Basic Realm"`,
+    scheme: `Basic`,
+    realm: `Basic Realm`,
+  });
+});
+
+add_task(async function unknown() {
+  await add_parse_realm_testcase({
+    input: `Newauth param="value"`,
+    scheme: `Basic`,
+    realm: ``,
+  });
+});
+
+add_task(async function parametersnotrequired() {
+  await add_parse_realm_testcase({ input: `A, B`, scheme: `Basic`, realm: `` });
+});
+
+add_task(async function disguisedrealm() {
+  await add_parse_realm_testcase({
+    input: `Basic foo="realm=nottherealm", realm="basic"`,
+    scheme: `Basic`,
+    realm: `basic`,
+  });
+});
+
+add_task(async function disguisedrealm2() {
+  await add_parse_realm_testcase({
+    input: `Basic nottherealm="nottherealm", realm="basic"`,
+    scheme: `Basic`,
+    realm: `basic`,
+  });
+});
+
+add_task(async function missingquote() {
+  await add_parse_realm_testcase({
+    input: `Basic realm="basic`,
+    scheme: `Basic`,
+    realm: `basic`,
+  });
+});