bug 846825 - refactor, make HSTS header parser more spec-conformant r=cviecco r=grobinson

netwerk/test/TestSTSParser.cpp

@@ -116,6 +116,10 @@ main(int32_t argc, char *argv[])
     rvs.AppendElement(TestSuccess("max-age  =100", false, 100, false, stss, pm));
     rvs.AppendElement(TestSuccess(" max-age=100", false, 100, false, stss, pm));
     rvs.AppendElement(TestSuccess("max-age = 100 ", false, 100, false, stss, pm));
+    rvs.AppendElement(TestSuccess("max-age = \"100\" ", false, 100, false, stss, pm));
+    rvs.AppendElement(TestSuccess("max-age=\"100\"", false, 100, false, stss, pm));
+    rvs.AppendElement(TestSuccess(" max-age =\"100\" ", false, 100, false, stss, pm));
+    rvs.AppendElement(TestSuccess("\tmax-age\t=\t\"100\"\t", false, 100, false, stss, pm));
     rvs.AppendElement(TestSuccess("max-age  =       100             ", false, 100, false, stss, pm));
 
     rvs.AppendElement(TestSuccess("maX-aGe=100", false, 100, false, stss, pm));
@@ -125,7 +129,7 @@ main(int32_t argc, char *argv[])
     rvs.AppendElement(TestSuccess("MAX-AGE = 100 ", false, 100, false, stss, pm));
 
     rvs.AppendElement(TestSuccess("max-age=100;includeSubdomains", false, 100, true, stss, pm));
-    rvs.AppendElement(TestSuccess("max-age=100; includeSubdomains", false, 100, true, stss, pm));
+    rvs.AppendElement(TestSuccess("max-age=100\t; includeSubdomains", false, 100, true, stss, pm));
     rvs.AppendElement(TestSuccess(" max-age=100; includeSubdomains", false, 100, true, stss, pm));
     rvs.AppendElement(TestSuccess("max-age = 100 ; includeSubdomains", false, 100, true, stss, pm));
     rvs.AppendElement(TestSuccess("max-age  =       100             ; includeSubdomains", false, 100, true, stss, pm));
@@ -135,13 +139,15 @@ main(int32_t argc, char *argv[])
     rvs.AppendElement(TestSuccess("max-AGE=100; iNcLuDeSuBdoMaInS", false, 100, true, stss, pm));
     rvs.AppendElement(TestSuccess("Max-Age = 100; includesubdomains ", false, 100, true, stss, pm));
     rvs.AppendElement(TestSuccess("INCLUDESUBDOMAINS;MaX-AgE = 100 ", false, 100, true, stss, pm));
+    // Turns out, the actual directive is entirely optional (hence the
+    // trailing semicolon)
+    rvs.AppendElement(TestSuccess("max-age=100;includeSubdomains;", true, 100, true, stss, pm));
 
     // these are weird tests, but are testing that some extended syntax is
     // still allowed (but it is ignored)
-    rvs.AppendElement(TestSuccess("max-age=100randomstuffhere", true, 100, false, stss, pm));
-    rvs.AppendElement(TestSuccess("max-age=100 includesubdomains", true, 100, false, stss, pm));
-    rvs.AppendElement(TestSuccess("max-age=100 bar foo", true, 100, false, stss, pm));
     rvs.AppendElement(TestSuccess("max-age=100 ; includesubdomainsSomeStuff", true, 100, false, stss, pm));
+    rvs.AppendElement(TestSuccess("\r\n\t\t    \tcompletelyUnrelated = foobar; max-age= 34520103    \t \t; alsoUnrelated;asIsThis;\tincludeSubdomains\t\t \t", true, 34520103, true, stss, pm));
+    rvs.AppendElement(TestSuccess("max-age=100; unrelated=\"quoted \\\"thingy\\\"\"", true, 100, false, stss, pm));
 
     rv0 = rvs.Contains(false) ? 1 : 0;
     if (rv0 == 0)
@@ -152,6 +158,7 @@ main(int32_t argc, char *argv[])
     // SHOULD FAIL:
     printf("*** Attempting to parse invalid STS headers (should not parse)...\n");
     // invalid max-ages
+    rvs.AppendElement(TestFailure("max-age", stss, pm));
     rvs.AppendElement(TestFailure("max-age ", stss, pm));
     rvs.AppendElement(TestFailure("max-age=p", stss, pm));
     rvs.AppendElement(TestFailure("max-age=*1p2", stss, pm));
@@ -166,6 +173,22 @@ main(int32_t argc, char *argv[])
     rvs.AppendElement(TestFailure("max-ag=100", stss, pm));
     rvs.AppendElement(TestFailure("includesubdomains", stss, pm));
     rvs.AppendElement(TestFailure(";", stss, pm));
+    rvs.AppendElement(TestFailure("max-age=\"100", stss, pm));
+    // The max-age directive here doesn't conform to the spec, so it MUST
+    // be ignored. Consequently, the REQUIRED max-age directive is not
+    // present in this header, and so it is invalid.
+    rvs.AppendElement(TestFailure("max-age=100, max-age=200; includeSubdomains", stss, pm));
+    rvs.AppendElement(TestFailure("max-age=100 includesubdomains", stss, pm));
+    rvs.AppendElement(TestFailure("max-age=100 bar foo", stss, pm));
+    rvs.AppendElement(TestFailure("max-age=100randomstuffhere", stss, pm));
+    // All directives MUST appear only once in an STS header field.
+    rvs.AppendElement(TestFailure("max-age=100; max-age=200", stss, pm));
+    rvs.AppendElement(TestFailure("includeSubdomains; max-age=200; includeSubdomains", stss, pm));
+    rvs.AppendElement(TestFailure("max-age=200; includeSubdomains; includeSubdomains", stss, pm));
+    // The includeSubdomains directive is valueless.
+    rvs.AppendElement(TestFailure("max-age=100; includeSubdomains=unexpected", stss, pm));
+    // LWS must have at least one space or horizontal tab
+    rvs.AppendElement(TestFailure("\r\nmax-age=200", stss, pm));
 
     rv1 = rvs.Contains(false) ? 1 : 0;
     if (rv1 == 0)

security/manager/boot/src/moz.build

@@ -10,6 +10,7 @@ CPP_SOURCES += [
     'nsBOOTModule.cpp',
     'nsEntropyCollector.cpp',
     'nsSecureBrowserUIImpl.cpp',
+    'nsSecurityHeaderParser.cpp',
     'nsSecurityWarningDialogs.cpp',
     'nsStrictTransportSecurityService.cpp',
 ]

security/manager/boot/src/nsSecurityHeaderParser.cpp

@@ -0,0 +1,254 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "nsSecurityHeaderParser.h"
+#include "prlog.h"
+
+// The character classes in this file are informed by [RFC2616], Section 2.2.
+// signed char is a signed data type one byte (8 bits) wide, so its value can
+// never be greater than 127. The following implicitly makes use of this.
+
+// A token is one or more CHAR except CTLs or separators.
+// A CHAR is any US-ASCII character (octets 0 - 127).
+// A CTL is any US-ASCII control character (octets 0 - 31) and DEL (127).
+// A separator is one of ()<>@,;:\"/[]?={} as well as space and
+// horizontal-tab (32 and 9, respectively).
+// So, this returns true if chr is any octet 33-126 except ()<>@,;:\"/[]?={}
+bool
+IsTokenSymbol(signed char chr) {
+  if (chr < 33 || chr == 127 ||
+      chr == '(' || chr == ')' || chr == '<' || chr == '>' ||
+      chr == '@' || chr == ',' || chr == ';' || chr == ':' ||
+      chr == '"' || chr == '/' || chr == '[' || chr == ']' ||
+      chr == '?' || chr == '=' || chr == '{' || chr == '}' || chr == '\\') {
+    return false;
+  }
+  return true;
+}
+
+// A quoted-string consists of a quote (") followed by any amount of
+// qdtext or quoted-pair, followed by a quote.
+// qdtext is any TEXT except a quote.
+// TEXT is any 8-bit octet except CTLs, but including LWS.
+// quoted-pair is a backslash (\) followed by a CHAR.
+// So, it turns out, \ can't really be a qdtext symbol for our purposes.
+// This returns true if chr is any octet 9,10,13,32-126 except <"> or "\"
+bool
+IsQuotedTextSymbol(signed char chr) {
+  return ((chr >= 32 && chr != '"' && chr != '\\' && chr != 127) ||
+          chr == 0x9 || chr == 0xa || chr == 0xd);
+}
+
+// The octet following the "\" in a quoted pair can be anything 0-127.
+bool
+IsQuotedPairSymbol(signed char chr) {
+  return (chr >= 0);
+}
+
+#if defined(PR_LOGGING)
+static PRLogModuleInfo *
+GetSHParserLog()
+{
+  static PRLogModuleInfo *sSHParserLog;
+  if (!sSHParserLog) {
+    sSHParserLog = PR_NewLogModule("nsSecurityHeaderParser");
+  }
+  return sSHParserLog;
+}
+#endif
+
+#define SHPARSERLOG(args) PR_LOG(GetSHParserLog(), PR_LOG_DEBUG, args)
+
+nsSecurityHeaderParser::nsSecurityHeaderParser(const char *aHeader)
+  : mCursor(aHeader)
+  , mError(false)
+{
+}
+
+nsSecurityHeaderParser::~nsSecurityHeaderParser() {
+  nsSecurityHeaderDirective *directive;
+  while ((directive = mDirectives.popFirst())) {
+    delete directive;
+  }
+}
+
+mozilla::LinkedList<nsSecurityHeaderDirective> *
+nsSecurityHeaderParser::GetDirectives() {
+  return &mDirectives;
+}
+
+nsresult
+nsSecurityHeaderParser::Parse() {
+  MOZ_ASSERT(mDirectives.isEmpty());
+  SHPARSERLOG(("trying to parse '%s'", mCursor));
+
+  Header();
+
+  // if we didn't consume the entire input, we were unable to parse it => error
+  if (mError || *mCursor) {
+    return NS_ERROR_FAILURE;
+  } else {
+    return NS_OK;
+  }
+}
+
+bool
+nsSecurityHeaderParser::Accept(char aChr)
+{
+  if (*mCursor == aChr) {
+    Advance();
+    return true;
+  }
+
+  return false;
+}
+
+bool
+nsSecurityHeaderParser::Accept(bool (*aClassifier) (signed char))
+{
+  if (aClassifier(*mCursor)) {
+    Advance();
+    return true;
+  }
+
+  return false;
+}
+
+void
+nsSecurityHeaderParser::Expect(char aChr)
+{
+  if (*mCursor != aChr) {
+    mError = true;
+  } else {
+    Advance();
+  }
+}
+
+void
+nsSecurityHeaderParser::Advance()
+{
+  // Technically, 0 is valid in quoted-pair, but we were handed a
+  // null-terminated const char *, so this doesn't handle that.
+  if (*mCursor) {
+    mOutput.Append(*mCursor);
+    mCursor++;
+  } else {
+    mError = true;
+  }
+}
+
+void
+nsSecurityHeaderParser::Header()
+{
+  Directive();
+  while (Accept(';')) {
+    Directive();
+  }
+}
+
+void
+nsSecurityHeaderParser::Directive()
+{
+  mDirective = new nsSecurityHeaderDirective();
+  LWSMultiple();
+  DirectiveName();
+  LWSMultiple();
+  if (Accept('=')) {
+    LWSMultiple();
+    DirectiveValue();
+    LWSMultiple();
+  }
+  mDirectives.insertBack(mDirective);
+  SHPARSERLOG(("read directive name '%s', value '%s'",
+               mDirective->mName.Data(), mDirective->mValue.Data()));
+}
+
+void
+nsSecurityHeaderParser::DirectiveName()
+{
+  mOutput.Truncate(0);
+  Token();
+  mDirective->mName.Assign(mOutput);
+}
+
+void
+nsSecurityHeaderParser::DirectiveValue()
+{
+  mOutput.Truncate(0);
+  if (Accept(IsTokenSymbol)) {
+    Token();
+    mDirective->mValue.Assign(mOutput);
+  } else if (Accept('"')) {
+    // Accept advances the cursor if successful, which appends a character to
+    // mOutput. The " is not part of what we want to capture, so truncate
+    // mOutput again.
+    mOutput.Truncate(0);
+    QuotedString();
+    mDirective->mValue.Assign(mOutput);
+    Expect('"');
+  }
+}
+
+void
+nsSecurityHeaderParser::Token()
+{
+  while (Accept(IsTokenSymbol));
+}
+
+void
+nsSecurityHeaderParser::QuotedString()
+{
+  while (true) {
+    if (Accept(IsQuotedTextSymbol)) {
+      QuotedText();
+    } else if (Accept('\\')) {
+      QuotedPair();
+    } else {
+      break;
+    }
+  }
+}
+
+void
+nsSecurityHeaderParser::QuotedText()
+{
+  while (Accept(IsQuotedTextSymbol));
+}
+
+void
+nsSecurityHeaderParser::QuotedPair()
+{
+  Accept(IsQuotedPairSymbol);
+}
+
+void
+nsSecurityHeaderParser::LWSMultiple()
+{
+  while (true) {
+    if (Accept('\r')) {
+      LWSCRLF();
+    } else if (Accept(' ') || Accept('\t')) {
+      LWS();
+    } else {
+      break;
+    }
+  }
+}
+
+void
+nsSecurityHeaderParser::LWSCRLF() {
+  Expect('\n');
+  if (!(Accept(' ') || Accept('\t'))) {
+    mError = true;
+  }
+  LWS();
+}
+
+void
+nsSecurityHeaderParser::LWS()
+{
+  // Note that becaue of how we're called, we don't have to check for
+  // the mandatory presense of at least one of SP or HT.
+  while (Accept(' ') || Accept('\t'));
+}

security/manager/boot/src/nsSecurityHeaderParser.h

@@ -0,0 +1,74 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef nsSecurityHeaderParser_h__
+#define nsSecurityHeaderParser_h__
+
+#include "nsString.h"
+#include "mozilla/LinkedList.h"
+#include "nsCOMPtr.h"
+
+// Utility class for handing back parsed directives and (optional) values
+class nsSecurityHeaderDirective : public mozilla::LinkedListElement<nsSecurityHeaderDirective> {
+public:
+  nsAutoCString mName;
+  nsAutoCString mValue;
+};
+
+// This class parses security-related HTTP headers like
+// Strict-Transport-Security. The Augmented Backus-Naur Form syntax for this
+// header is reproduced below, for reference:
+//
+//   Strict-Transport-Security = "Strict-Transport-Security" ":"
+//                               [ directive ]  *( ";" [ directive ] )
+//
+//   directive                 = directive-name [ "=" directive-value ]
+//   directive-name            = token
+//   directive-value           = token | quoted-string
+//
+//   where:
+//
+//   token          = <token, defined in [RFC2616], Section 2.2>
+//   quoted-string  = <quoted-string, defined in [RFC2616], Section 2.2>/
+//
+// For further reference, see [RFC6797], Section 6.1
+
+class nsSecurityHeaderParser {
+public:
+  nsSecurityHeaderParser(const char *aHeader);
+  ~nsSecurityHeaderParser();
+
+  // Only call Parse once.
+  nsresult Parse();
+  // The caller does not take ownership of the memory returned here.
+  mozilla::LinkedList<nsSecurityHeaderDirective> *GetDirectives();
+
+private:
+  bool Accept(char aChr);
+  bool Accept(bool (*aClassifier) (signed char));
+  void Expect(char aChr);
+  void Advance();
+  void Header();         // header = [ directive ] *( ";" [ directive ] )
+  void Directive();      // directive = directive-name [ "=" directive-value ]
+  void DirectiveName();  // directive-name = token
+  void DirectiveValue(); // directive-value = token | quoted-string
+  void Token();          // token = 1*<any CHAR except CTLs or separators>
+  void QuotedString();   // quoted-string = (<"> *( qdtext | quoted-pair ) <">)
+  void QuotedText();     // qdtext = <any TEXT except <"> and "\">
+  void QuotedPair();     // quoted-pair = "\" CHAR
+
+                         // LWS = [CRLF] 1*( SP | HT )
+  void LWSMultiple();    // Handles *( LWS )
+  void LWSCRLF();        // Handles the [CRLF] part of LWS
+  void LWS();            // Handles the 1*( SP | HT ) part of LWS
+
+  mozilla::LinkedList<nsSecurityHeaderDirective> mDirectives;
+  const char *mCursor;
+  nsSecurityHeaderDirective *mDirective;
+
+  nsAutoCString mOutput;
+  bool mError;
+};
+
+#endif /* nsSecurityHeaderParser_h__ */

security/manager/boot/src/nsStrictTransportSecurityService.cpp

@@ -17,6 +17,8 @@
 #include "nsIScriptSecurityManager.h"
 #include "nsISocketProvider.h"
 #include "mozilla/Preferences.h"
+#include "mozilla/LinkedList.h"
+#include "nsSecurityHeaderParser.h"
 
 // A note about the preload list:
 // When a site specifically disables sts by sending a header with
@@ -45,12 +47,6 @@ GetSTSLog()
 
 #define STSLOG(args) PR_LOG(GetSTSLog(), 4, args)
 
-#define STS_PARSER_FAIL_IF(test,args) \
-  if (test) { \
-    STSLOG(args); \
-    return NS_ERROR_FAILURE; \
-  }
-
 ////////////////////////////////////////////////////////////////////////////////
 
 nsSTSHostEntry::nsSTSHostEntry(const char* aHost)
@@ -252,7 +248,7 @@ nsStrictTransportSecurityService::ProcessStsHeaderMutating(nsIURI* aSourceURI,
                                                            uint64_t *aMaxAge,
                                                            bool *aIncludeSubdomains)
 {
-  STSLOG(("STS: ProcessStrictTransportHeader(%s)\n", aHeader));
+  STSLOG(("STS: processing header '%s'", aHeader));
 
   // "Strict-Transport-Security" ":" OWS
   //      STS-d  *( OWS ";" OWS STS-d  OWS)
@@ -263,95 +259,100 @@ nsStrictTransportSecurityService::ProcessStsHeaderMutating(nsIURI* aSourceURI,
   //  maxAge     = "max-age" "=" delta-seconds v-ext
   //
   //  includeSubDomains = [ "includeSubDomains" ]
-
+  //
-  const char* directive;
+  //  The order of the directives is not significant.
+  //  All directives must appear only once.
+  //  Directive names are case-insensitive.
+  //  The entire header is invalid if a directive not conforming to the
+  //  syntax is encountered.
+  //  Unrecognized directives (that are otherwise syntactically valid) are
+  //  ignored, and the rest of the header is parsed as normal.
 
   bool foundMaxAge = false;
-  bool foundUnrecognizedTokens = false;
+  bool foundIncludeSubdomains = false;
-  bool includeSubdomains = false;
+  bool foundUnrecognizedDirective = false;
   int64_t maxAge = 0;
 
   NS_NAMED_LITERAL_CSTRING(max_age_var, "max-age");
   NS_NAMED_LITERAL_CSTRING(include_subd_var, "includesubdomains");
 
-  while ((directive = NS_strtok(";", &aHeader))) {
-    //skip leading whitespace
-    directive = NS_strspnp(" \t", directive);
-    STS_PARSER_FAIL_IF(!(*directive), ("error removing initial whitespace\n."));
 
-    if (!PL_strncasecmp(directive, max_age_var.get(), max_age_var.Length())) {
+  nsSecurityHeaderParser parser(aHeader);
-      // skip directive name
+  nsresult rv = parser.Parse();
-      directive += max_age_var.Length();
+  if (NS_FAILED(rv)) {
-      // skip leading whitespace
+    STSLOG(("STS: could not parse header"));
-      directive = NS_strspnp(" \t", directive);
+    return rv;
-      STS_PARSER_FAIL_IF(*directive != '=',
+  }
-                  ("No equal sign found in max-age directive\n"));
+  mozilla::LinkedList<nsSecurityHeaderDirective> *directives = parser.GetDirectives();
 
-      // skip over the equal sign
+  for (nsSecurityHeaderDirective *directive = directives->getFirst();
-      STS_PARSER_FAIL_IF(*(++directive) == '\0',
+       directive != nullptr; directive = directive->getNext()) {
-                  ("No delta-seconds present\n"));
+    if (directive->mName.Length() == max_age_var.Length() &&
+        directive->mName.EqualsIgnoreCase(max_age_var.get(),
+                                          max_age_var.Length())) {
+      if (foundMaxAge) {
+        STSLOG(("STS: found two max-age directives"));
+        return NS_ERROR_FAILURE;
+      }
 
-      // obtain the delta-seconds value
+      STSLOG(("STS: found max-age directive"));
-      STS_PARSER_FAIL_IF(PR_sscanf(directive, "%lld", &maxAge) != 1,
-                  ("Could not convert delta-seconds\n"));
-      STSLOG(("STS: ProcessStrictTransportHeader() STS found maxage %lld\n", maxAge));
       foundMaxAge = true;
 
-      // skip max-age value and trailing whitespace
+      size_t len = directive->mValue.Length();
-      directive = NS_strspnp("0123456789 \t", directive);
+      for (size_t i = 0; i < len; i++) {
-
+        char chr = directive->mValue.CharAt(i);
-      // log unknown tokens, but don't fail (for forwards compatibility)
+        if (chr < '0' || chr > '9') {
-      if (*directive != '\0') {
+          STSLOG(("STS: invalid value for max-age directive"));
-        foundUnrecognizedTokens = true;
+          return NS_ERROR_FAILURE;
-        STSLOG(("Extra stuff in max-age after delta-seconds: %s \n", directive));
-      }
-    }
-    else if (!PL_strncasecmp(directive, include_subd_var.get(), include_subd_var.Length())) {
-      directive += include_subd_var.Length();
-
-      // only record "includesubdomains" if it is a token by itself... for
-      // example, don't set includeSubdomains = true if the directive is
-      // "includesubdomainsFooBar".
-      if (*directive == '\0' || *directive =='\t' || *directive == ' ') {
-        includeSubdomains = true;
-        STSLOG(("STS: ProcessStrictTransportHeader: obtained subdomains status\n"));
-
-        // skip trailing whitespace
-        directive = NS_strspnp(" \t", directive);
-
-        if (*directive != '\0') {
-          foundUnrecognizedTokens = true;
-          STSLOG(("Extra stuff after includesubdomains: %s\n", directive));
         }
-      } else {
-        foundUnrecognizedTokens = true;
-        STSLOG(("Unrecognized directive in header: %s\n", directive));
       }
-    }
+
-    else {
+      if (PR_sscanf(directive->mValue.get(), "%lld", &maxAge) != 1) {
-      // log unknown directives, but don't fail (for backwards compatibility)
+        STSLOG(("STS: could not parse delta-seconds"));
-      foundUnrecognizedTokens = true;
+        return NS_ERROR_FAILURE;
-      STSLOG(("Unrecognized directive in header: %s\n", directive));
+      }
+
+      STSLOG(("STS: parsed delta-seconds: %lld", maxAge));
+    } else if (directive->mName.Length() == include_subd_var.Length() &&
+               directive->mName.EqualsIgnoreCase(include_subd_var.get(),
+                                                 include_subd_var.Length())) {
+      if (foundIncludeSubdomains) {
+        STSLOG(("STS: found two includeSubdomains directives"));
+        return NS_ERROR_FAILURE;
+      }
+
+      STSLOG(("STS: found includeSubdomains directive"));
+      foundIncludeSubdomains = true;
+
+      if (directive->mValue.Length() != 0) {
+        STSLOG(("STS: includeSubdomains directive unexpectedly had value '%s'", directive->mValue.get()));
+        return NS_ERROR_FAILURE;
+      }
+    } else {
+      STSLOG(("STS: ignoring unrecognized directive '%s'", directive->mName.get()));
+      foundUnrecognizedDirective = true;
     }
   }
 
   // after processing all the directives, make sure we came across max-age
   // somewhere.
-  STS_PARSER_FAIL_IF(!foundMaxAge,
+  if (!foundMaxAge) {
-              ("Parse ERROR: couldn't locate max-age token\n"));
+    STSLOG(("STS: did not encounter required max-age directive"));
+    return NS_ERROR_FAILURE;
+  }
 
   // record the successfully parsed header data.
-  SetStsState(aSourceURI, maxAge, includeSubdomains, aFlags);
+  SetStsState(aSourceURI, maxAge, foundIncludeSubdomains, aFlags);
 
   if (aMaxAge != nullptr) {
     *aMaxAge = (uint64_t)maxAge;
   }
 
   if (aIncludeSubdomains != nullptr) {
-    *aIncludeSubdomains = includeSubdomains;
+    *aIncludeSubdomains = foundIncludeSubdomains;
   }
 
-  return foundUnrecognizedTokens ?
+  return foundUnrecognizedDirective ?
          NS_SUCCESS_LOSS_OF_INSIGNIFICANT_DATA :
          NS_OK;
 }

security/manager/ssl/tests/unit/test_sts_preloadlist_perwindowpb.js

@@ -163,13 +163,12 @@ function test_private_browsing1() {
   // (sanity check first - this should be in the preload list)
   do_check_true(gSTSService.isStsHost("login.persona.org", IS_PRIVATE));
   var uri = Services.io.newURI("http://login.persona.org", null, null);
-  // according to the rfc, max-age can't be negative, but this is a great
+  gSTSService.processStsHeader(uri, "max-age=1", IS_PRIVATE);
-  // way to test an expired entry
+  do_timeout(1250, function() {
-  gSTSService.processStsHeader(uri, "max-age=-1000", IS_PRIVATE);
+    do_check_false(gSTSService.isStsHost("login.persona.org", IS_PRIVATE));
-  do_check_false(gSTSService.isStsHost("login.persona.org", IS_PRIVATE));
+    // Simulate leaving private browsing mode
-
+    Services.obs.notifyObservers(null, "last-pb-context-exited", null);
-  // Simulate leaving private browsing mode
+  });
-  Services.obs.notifyObservers(null, "last-pb-context-exited", null);
 }
 
 function test_private_browsing2() {