Extensions: Consolidate and simplify CSP parsing.

This CL adds a csp_validator::ParseCSP method to parse a CSP string. Existing
code doing the parsing is changed to use this new method.

This CL should have no behavior change. Also add unit tests exercising the new
method.

BUG=896041

Change-Id: Ic97e31ab23b6d21c0b3ad4b771a2fd7c37bec6d4
Reviewed-on: https://chromium-review.googlesource.com/c/1292825
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602224}

extensions/common/csp_validator.h

@@ -6,7 +6,10 @@
 #define EXTENSIONS_COMMON_CSP_VALIDATOR_H_
 
 #include <string>
+#include <vector>
 
+#include "base/macros.h"
+#include "base/strings/string_piece_forward.h"
 #include "extensions/common/manifest.h"
 
 namespace extensions {
@@ -31,6 +34,60 @@ enum Options {
   OPTIONS_ALLOW_INSECURE_OBJECT_SRC = 1 << 1,
 };
 
+// Helper to parse a serialized content security policy string.
+// Exposed for testing.
+class CSPParser {
+ public:
+  // Represents a CSP directive.
+  // E.g. for the directive "script-src 'self' www.google.com"
+  // |directive_string| is "script-src 'self' www.google.com".
+  // |directive_name| is "script_src".
+  // |directive_values| is ["'self'", "www.google.com"].
+  struct Directive {
+    Directive(base::StringPiece directive_string,
+              std::string directive_name,
+              std::vector<base::StringPiece> directive_values);
+    ~Directive();
+    Directive(Directive&&);
+    Directive& operator=(Directive&&);
+
+    base::StringPiece directive_string;
+
+    // Must be lower case.
+    std::string directive_name;
+
+    std::vector<base::StringPiece> directive_values;
+
+    DISALLOW_COPY_AND_ASSIGN(Directive);
+  };
+
+  using DirectiveList = std::vector<Directive>;
+
+  CSPParser(std::string policy);
+  ~CSPParser();
+
+  // It's not safe to move CSPParser since |directives_| refers to memory owned
+  // by |policy_|. Once move constructed, |directives_| will end up being in an
+  // invalid state, as it will point to memory owned by a "moved" string
+  // instance.
+  CSPParser(CSPParser&&) = delete;
+  CSPParser& operator=(CSPParser&&) = delete;
+
+  // This can contain duplicate directives (directives having the same directive
+  // name).
+  const DirectiveList& directives() const { return directives_; }
+
+ private:
+  void Parse();
+
+  const std::string policy_;
+
+  // This refers to memory owned by |policy_|.
+  DirectiveList directives_;
+
+  DISALLOW_COPY_AND_ASSIGN(CSPParser);
+};
+
 // Checks whether the given |policy| meets the minimum security requirements
 // for use in the extension system.
 //

extensions/common/csp_validator_unittest.cc

@@ -5,6 +5,8 @@
 #include <stddef.h>
 
 #include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "extensions/common/csp_validator.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/install_warning.h"
@@ -534,3 +536,59 @@ TEST(ExtensionCSPValidator, EffectiveSandboxedPageCSP) {
       InsecureValueWarning("child-src", "http://bar.com"),
       InsecureValueWarning("child-src", "http://foo.com")));
 }
+
+namespace extensions {
+namespace csp_validator {
+
+void PrintTo(const CSPParser::Directive& directive, ::std::ostream* os) {
+  *os << base::StringPrintf(
+      "[[%s] [%s] [%s]]", directive.directive_string.as_string().c_str(),
+      directive.directive_name.c_str(),
+      base::JoinString(directive.directive_values, ",").c_str());
+}
+
+}  // namespace csp_validator
+}  // namespace extensions
+
+TEST(ExtensionCSPValidator, ParseCSP) {
+  using CSPParser = extensions::csp_validator::CSPParser;
+  using DirectiveList = CSPParser::DirectiveList;
+
+  struct TestCase {
+    TestCase(const char* policy, DirectiveList expected_directives)
+        : policy(policy), expected_directives(std::move(expected_directives)) {}
+    const char* policy;
+    DirectiveList expected_directives;
+  };
+
+  std::vector<TestCase> cases;
+
+  cases.emplace_back("   \n \r \t ", DirectiveList());
+  cases.emplace_back("  ; \n ;\r \t ;;", DirectiveList());
+
+  const char* policy = R"(  deFAULt-src   'self' ;
+  img-src * ; media-src media1.com MEDIA2.com;
+  img-src 'self';
+  )";
+  DirectiveList expected_directives;
+  expected_directives.emplace_back("deFAULt-src   'self'", "default-src",
+                                   std::vector<base::StringPiece>({"'self'"}));
+  expected_directives.emplace_back("img-src *", "img-src",
+                                   std::vector<base::StringPiece>({"*"}));
+  expected_directives.emplace_back(
+      "media-src media1.com MEDIA2.com", "media-src",
+      std::vector<base::StringPiece>({"media1.com", "MEDIA2.com"}));
+  expected_directives.emplace_back("img-src 'self'", "img-src",
+                                   std::vector<base::StringPiece>({"'self'"}));
+  cases.emplace_back(policy, std::move(expected_directives));
+
+  for (const auto& test_case : cases) {
+    SCOPED_TRACE(test_case.policy);
+
+    CSPParser parser(test_case.policy);
+
+    // Cheat and compare serialized versions of the directives.
+    EXPECT_EQ(::testing::PrintToString(parser.directives()),
+              ::testing::PrintToString(test_case.expected_directives));
+  }
+}