Add HTTPMultipartBuilder and its test.

util/net/http_multipart_builder.h

+// Copyright 2014 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef CRASHPAD_UTIL_NET_HTTP_MULTIPART_BUILDER_H_
+#define CRASHPAD_UTIL_NET_HTTP_MULTIPART_BUILDER_H_
+
+#include "base/basictypes.h"
+#include "base/files/file_path.h"
+#include "base/memory/scoped_ptr.h"
+
+#include <map>

[Mark Mentovai, 2014/10/28 22:34:28]

C++ system headers should precede project-local headers.

[Robert Sesek, 2014/10/29 19:52:01]

Done.

+#include <string>
+
+namespace crashpad {
+
+class HTTPBodyStream;
+
+//! \brief This class is used to build a MIME multipart message, conforming to
+//!     RFC 2046, for use as a HTTP request body.
+class HTTPMultipartBuilder {
+ public:
+  HTTPMultipartBuilder();
+  ~HTTPMultipartBuilder();
+
+  //! \brief Sets a `Content-Disposition: form-data` key-value pair.
+  //!
+  //! \param[in] key The key of the form data, specified as the `name` in the
+  //!     multipart message.
+  //! \param[in] value The value to set at the \a key. This will overwrite any
+  //!     previously set value with the specified key.
+  void SetFormData(const std::string& key, const std::string& value);

[Mark Mentovai, 2014/10/28 22:34:28]

|key| in this method and [the key, see below] in SetFileAttachment() should
share a namespace. HTTP doesn’t require it, but HTTP doesn‘t actually require
unique keys at all. You’ve decided to ensure unique keys in your implementation,
which is a perfectly reasonable thing to do, so I think you should also ensure
uniqueness among |key| and [the key used by SetFileAttachment()].

This should be documented (since you’ve gone to the trouble of documenting |key|
uniqueness, which is good).

The implementation itself is unaware that the namespace is shared, and will
actually output two parts with identical names. I don’t think that this is wise.

You may be able to do this pretty easily by tracking everything in a single
std::map<std::string, HTTPBodyStream*> instead of having two
std::map<std::string, …>s.

[Robert Sesek, 2014/10/29 19:52:01]

Right, I was playing both sides of this coin by enforcing uniqueness in each of
the individual keyspaces, while relying on HTTP to not care if there is a
collision between the two.
This class cannot own HTTPBodyStream objects because they are passed to the
CompositeHTTPBodyStream, which takes ownership. I'd either have to add a copy
ctor to HTTPBodyStream, or have GetBodyStream() also clear all state on this
object. I do now enforce uniqueness in both keyspaces, though.

+
+  //! \brief Specifies the file at \a path to have its contents uploaded as
+  //!     multipart data, available at `name` of \a upload_file_name.

[Mark Mentovai, 2014/10/28 22:34:28]

I think this should be a 3-argument call: (const std::string& key, const
std::string& upload_file_name, const base::FilePath& path).

|upload_file_name| should appear at filename in the HTTP part. See RFC 2388 §4.4
and blink/Source/platform/network/FormDataBuilder.cpp
blink::FormDataBuilder::addFilenameToMultiPartHeader(). You could do this with a
2-argument form by using path.value() to infer filename, but I don’t think
that’s so hot. It’s reasonable to want to “lie” about the filename to the
server, because what’s on disk might be some temporary meaningless name. (In
addition, it’s reasonable to require upload_file_name to just be a basename, or
to take the basename of whatever’s passed in. Full paths aren’t really
appropriate.)

|key| is distinct from |upload_file_name|. In the HTTP part, you’ll get
name="key" and filename="upload_file_name".

Optionally, you could also allow a content_type parameter here, or you could
ignore it and always write Content-Type: application/octet-stream (RFC 2388
§4.1).

[Robert Sesek, 2014/10/29 19:52:01]

Done.

+  //!
+  //! \param[in] upload_file_name The name to give this multipart data. If a
+  //!     \a path has already been supplied at this name, a subsequent call
+  //!     will overwrite the path with the newly specified one. This is used
+  //!     as the `name` in the multipart message.
+  //! \param[in] path The path of the files whose contents will be uploaded.
+  void SetFileAttachment(const std::string& upload_file_name,
+                         const base::FilePath& path);
+
+  //! \brief Generates the HTTPBodyStream for the data currently supplied to
+  //!     the builder.
+  //!
+  //! \return A caller-owned HTTPBodyStream object.
+  scoped_ptr<HTTPBodyStream> GetBodyStream();
+
+ private:
+  // Generates a random string suitable for use as a multipart boundary.
+  static std::string GenerateBoundaryString();

[Mark Mentovai, 2014/10/28 22:34:28]

Is there even a reason to expose these private static methods in the class at
all? They can just be anonymous-namespace functions in
http_multipart_builder.cc.

[Robert Sesek, 2014/10/29 19:52:01]

No, moved to anon.

+
+  // Escapes the specified name to be suitable for the name field of a
+  // form-data part.
+  static std::string EncodeFieldName(const std::string& name);
+
+  // Returns a string, formatted with a multipart boundary and a field name,
+  // after which the contents of the part at |name| can be appended.
+  static std::string GetFormDataBoundary(const std::string& boundary,
+                                         const std::string& name);
+
+  std::map<std::string, std::string> form_data_;
+  std::map<std::string, base::FilePath> file_attachments_;
+
+  DISALLOW_COPY_AND_ASSIGN(HTTPMultipartBuilder);
+};
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_UTIL_NET_HTTP_MULTIPART_BUILDER_H_