Add read-only access to POST data for webRequest's onBeforeRequest

chrome/browser/extensions/api/web_request/post_data_parser.h

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_API_WEB_REQUEST_POST_DATA_PARSER_H_
+#define CHROME_BROWSER_EXTENSIONS_API_WEB_REQUEST_POST_DATA_PARSER_H_
+
+#include <vector>
+
+#include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/string_piece.h"
+
+namespace net {
+class URLRequest;
+}
+
+namespace extensions {
+
+// Interface for parsers for the POST data.
+class PostDataParser {

[battre, 2012/07/13 15:20:04]

now that we have separated this into separate files, I think there should be
some unit tests in post_data_parser_unittest.cc, see my comment in
web_request_api_unittest.cc.

[vabr (Chromium), 2012/07/16 15:40:51]

Done.

+ public:
+  class Result {
+   public:
+    Result();

[battre, 2012/07/13 15:20:04]

you need to define a destructor, otherwise the destructor is generated multiple
times (wired C++ behavior).

[vabr (Chromium), 2012/07/16 15:40:51]

Done.

+    std::string& get_key() {

[battre, 2012/07/13 15:20:04]

I would remove this and the following function and rely on SetKey, this is more
common.

[vabr (Chromium), 2012/07/16 15:40:51]

Done.

+      return key_;
+    }
+    std::string& get_val() {
+      return val_;
+    }
+    const std::string& get_key() const {
+      return key_;
+    }
+    const std::string& get_val() const {
+      return val_;
+    }
+    void Reset();
+    void SetKey(const char* s, size_t n);

[battre, 2012/07/13 15:20:04]

I would change this to
void set_key(const std::string& key) {
  key_ = key;
}
same below. The current implementation looks complicated.

[vabr (Chromium), 2012/07/16 15:40:51]

After speaking to Dominic, I added a new version of SetKey, taking std::string,
in addition to the original version, which aimed at sparing one copying pass.

+    void SetVal(const char* s, size_t n);
+
+   private:
+    std::string key_;
+    std::string val_;
+  };
+  // Creates a correct parser instance based on the |request|. Returns NULL
+  // on failure.
+  static scoped_ptr<PostDataParser> CreatePostDataParser(
+      net::URLRequest* request);
+  // No member variables, so the destructor can be safely inlined.
+  virtual ~PostDataParser() {}

[battre, 2012/07/13 15:20:04]

no, our rule is that virtual functions should be never inlined.

[vabr (Chromium), 2012/07/16 15:40:51]

Done (and especially ashamed about this one).

+  // Returns true if there was some data, it was well formed and all was read.
+  virtual bool AllDataReadOK() = 0;
+  // Returns the next key-value pair as |result|. After SetSource has succeeded,
+  // this allows to iterate over all pairs in the source.
+  // Returns true as long as a new pair was successfully found.
+  virtual bool GetNextPair(Result* result) = 0;
+  // Sets the |source| of the data to be parsed. The ownership is let with the
+  // caller and the source should live until |this| dies or |this->SetSource()|
+  // is called again, whatever comes sooner. Returns true on success.
+  virtual bool SetSource(const std::vector<char>* source) = 0;
+
+ protected:
+  // No member variables, so the constructor can be safely inlined.
+  PostDataParser() {}

[battre, 2012/07/13 15:20:04]

clang might complain about that.

[vabr (Chromium), 2012/07/16 15:40:51]

The trybot linux_clang did not issue a warning about this. If you still want me
to put this into .cc, let me know.

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PostDataParser);
+};
+
+// Parses URLencoded forms, see
+// http://www.w3.org/TR/REC-html40-971218/interact/forms.html#h-17.13.4.1 .
+class PostDataParserUrlEncoded : public PostDataParser {
+ public:
+  PostDataParserUrlEncoded();
+  virtual ~PostDataParserUrlEncoded();
+  // Implementation of PostDataParser.
+  virtual bool AllDataReadOK() OVERRIDE;
+  virtual bool GetNextPair(Result* result) OVERRIDE;
+  virtual bool SetSource(const std::vector<char>* source) OVERRIDE;
+
+ private:
+  const std::vector<char>* source_;
+  std::vector<char>::const_iterator offset_;
+  DISALLOW_COPY_AND_ASSIGN(PostDataParserUrlEncoded);
+};
+
+// Parses forms encoded as multipart, see RFC2388.
+class PostDataParserMultipart : public PostDataParser {
+ public:
+  explicit PostDataParserMultipart(const std::string& boundary_separator);
+  virtual ~PostDataParserMultipart();
+  // Implementation of PostDataParser.
+  virtual bool AllDataReadOK() OVERRIDE;
+  virtual bool GetNextPair(Result* result) OVERRIDE;
+  virtual bool SetSource(const std::vector<char>* source) OVERRIDE;
+
+ private:
+  // Note on implementation:
+  // This parser reads the source line by line. There are four types of lines:
+  // - "Boundary" line, separating entries.
+  // - "End boundary" line, ends the data.
+  // - "Content-Disposition" line, containing the "key" for |result|.
+  // - Empty lines.
+  // - Other non-empty lines.
+  enum Lines {kBoundary, kEndBoundary, kDisposition, kEmpty, kOther};
+  // The parser uses the following 7-state automaton to check for structure:
+  // kInit --Boundary--> kFirst, kInit --not(Boundary)--> kError
+  // kFirst --Disposition--> kSkip, kFirst --not(Disposition)--> kHead
+  // kHead --Disposition--> kSkip, kHead --not(Disposition)-->kHead
+  // kSkip --Empty-->kBody, kSkip --not(Empty)--> kSkip
+  // kBody --Boundary--> kFirst, kBody --EndBoundary--> kFinal,
+  //      kBody --not(Boundary)--> kBody
+  enum States {kInit, kFirst, kHead, kSkip, kBody, kFinal, kError};
+  // One-step of the automaton, based on |state_| and |line_type_|.
+  // This calls GetNextLine() and returns it return value.
+  bool DoStep();
+  // Determine the |line_type_| of the current line_start_.
+  void GetLineType();
+  // Read one more line from |source_|, update line pointers.
+  bool GetNextLine();
+  // Extracts "key" and possibly "val" from a DISP line. Returns success.
+  bool ParseHead(Result* result, bool* val_extracted);
+  // We parse the first |length_| bytes from |source_|.
+  const char* source_;
+  size_t length_;
+  // Offset of the current and next line from |source_|:
+  // [line_start_]...line... [line_end_]EOL [next_line_]...line...
+  size_t line_start_;
+  size_t line_end_;
+  size_t next_line_;
+  const std::string boundary_;

[battre, 2012/07/13 15:20:04]

Add #include <string> for this.

[vabr (Chromium), 2012/07/16 15:40:51]

Done.

+  const std::string final_boundary_;
+  States state_;
+  Lines line_type_;
+  DISALLOW_COPY_AND_ASSIGN(PostDataParserMultipart);
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_EXTENSIONS_API_WEB_REQUEST_POST_DATA_PARSER_H_