Merge pull request #492 from sandstorm-io/tweaks

Tweaks and bugfixes

c++/src/kj/array-test.c++

@@ -378,5 +378,12 @@ TEST(Array, ReleaseAsBytesOrChars) {
   }
 }
 
+#if __cplusplus > 201402L
+KJ_TEST("kj::arr()") {
+  kj::Array<kj::String> array = kj::arr(kj::str("foo"), kj::str(123));
+  KJ_EXPECT(array == kj::ArrayPtr<const kj::StringPtr>({"foo", "123"}));
+}
+#endif
+
 }  // namespace
 }  // namespace kj

c++/src/kj/array.h

@@ -177,6 +177,11 @@ public:
   inline T& front() { return *ptr; }
   inline T& back() { return *(ptr + size_ - 1); }
 
+  template <typename U>
+  inline bool operator==(const U& other) const { return asPtr() == other; }
+  template <typename U>
+  inline bool operator!=(const U& other) const { return asPtr() != other; }
+
   inline ArrayPtr<T> slice(size_t start, size_t end) {
     KJ_IREQUIRE(start <= end && end <= size_, "Out-of-bounds Array::slice().");
     return ArrayPtr<T>(ptr + start, end - start);
@@ -249,6 +254,8 @@ private:
 
   template <typename U>
   friend class Array;
+  template <typename U>
+  friend class ArrayBuilder;
 };
 
 static_assert(!canMemcpy<Array<char>>(), "canMemcpy<>() is broken");
@@ -321,6 +328,13 @@ public:
     other.pos = nullptr;
     other.endPtr = nullptr;
   }
+  ArrayBuilder(Array<T>&& other)
+      : ptr(other.ptr), pos(other.ptr + other.size_), endPtr(pos), disposer(other.disposer) {
+    // Create an already-full ArrayBuilder from an Array of the same type. This constructor
+    // primarily exists to enable Vector<T> to be constructed from Array<T>.
+    other.ptr = nullptr;
+    other.size_ = 0;
+  }
   KJ_DISALLOW_COPY(ArrayBuilder);
   inline ~ArrayBuilder() noexcept(false) { dispose(); }
 
@@ -808,6 +822,15 @@ inline Array<T> heapArray(std::initializer_list<T> init) {
   return heapArray<T>(init.begin(), init.end());
 }
 
+#if __cplusplus > 201402L
+template <typename T, typename... Params>
+inline Array<Decay<T>> arr(T&& param1, Params&&... params) {
+  ArrayBuilder<Decay<T>> builder = heapArrayBuilder<Decay<T>>(sizeof...(params) + 1);
+  (builder.add(kj::fwd<T>(param1)), ... , builder.add(kj::fwd<Params>(params)));
+  return builder.finish();
+}
+#endif
+
 }  // namespace kj
 
 #endif  // KJ_ARRAY_H_

c++/src/kj/async-io.h

@@ -86,8 +86,9 @@ class AsyncOutputStream {
   // Asynchronous equivalent of OutputStream (from io.h).
 
 public:
-  virtual Promise<void> write(const void* buffer, size_t size) = 0;
-  virtual Promise<void> write(ArrayPtr<const ArrayPtr<const byte>> pieces) = 0;
+  virtual Promise<void> write(const void* buffer, size_t size) KJ_WARN_UNUSED_RESULT = 0;
+  virtual Promise<void> write(ArrayPtr<const ArrayPtr<const byte>> pieces)
+      KJ_WARN_UNUSED_RESULT = 0;
 
   virtual Maybe<Promise<uint64_t>> tryPumpFrom(
       AsyncInputStream& input, uint64_t amount = kj::maxValue);

c++/src/kj/compat/http.c++

@@ -1297,7 +1297,7 @@ public:
     KJ_REQUIRE(inBody) { return kj::READY_NOW; }
 
     auto fork = writeQueue.then([this,buffer,size]() {
-      inner.write(buffer, size);
+      return inner.write(buffer, size);
     }).fork();
 
     writeQueue = fork.addBranch();
@@ -1308,7 +1308,7 @@ public:
     KJ_REQUIRE(inBody) { return kj::READY_NOW; }
 
     auto fork = writeQueue.then([this,pieces]() {
-      inner.write(pieces);
+      return inner.write(pieces);
     }).fork();
 
     writeQueue = fork.addBranch();

c++/src/kj/compat/url.c++

@@ -171,42 +171,36 @@ Maybe<Url> Url::tryParse(StringPtr text, Context context) {
     }
   }
 
-  {
-    Vector<String> path;
-    while (text.startsWith("/")) {
-      text = text.slice(1);
-      auto part = split(text, END_PATH_PART);
-      if (part.size() == 2 && part[0] == '.' && part[1] == '.') {
-        if (path.size() != 0) {
-          path.removeLast();
-        }
-        result.hasTrailingSlash = true;
-      } else if (part.size() == 0 || (part.size() == 1 && part[0] == '.')) {
-        // Collapse consecutive slashes and "/./".
-        result.hasTrailingSlash = true;
-      } else {
-        path.add(percentDecode(part, err));
-        result.hasTrailingSlash = false;
+  while (text.startsWith("/")) {
+    text = text.slice(1);
+    auto part = split(text, END_PATH_PART);
+    if (part.size() == 2 && part[0] == '.' && part[1] == '.') {
+      if (result.path.size() != 0) {
+        result.path.removeLast();
       }
+      result.hasTrailingSlash = true;
+    } else if (part.size() == 0 || (part.size() == 1 && part[0] == '.')) {
+      // Collapse consecutive slashes and "/./".
+      result.hasTrailingSlash = true;
+    } else {
+      result.path.add(percentDecode(part, err));
+      result.hasTrailingSlash = false;
     }
-    result.path = path.releaseAsArray();
   }
 
   if (text.startsWith("?")) {
-    Vector<QueryParam> params;
     do {
       text = text.slice(1);
       auto part = split(text, END_QUERY_PART);
 
       if (part.size() > 0) {
         KJ_IF_MAYBE(key, trySplit(part, '=')) {
-          params.add(QueryParam { percentDecode(*key, err), percentDecode(part, err) });
+          result.query.add(QueryParam { percentDecode(*key, err), percentDecode(part, err) });
         } else {
-          params.add(QueryParam { percentDecode(part, err), nullptr });
+          result.query.add(QueryParam { percentDecode(part, err), nullptr });
         }
       }
     } while (text.startsWith("&"));
-    result.query = params.releaseAsArray();
   }
 
   if (text.startsWith("#")) {
@@ -293,7 +287,6 @@ Maybe<Url> Url::tryParseRelative(StringPtr text) const {
   bool hadNewPath = text.size() > 0 && text[0] != '?' && text[0] != '#';
   if (hadNewPath) {
     // There's a new path.
-    Vector<String> path(this->path.size());
 
     if (text[0] == '/') {
       // New path is absolute, so don't copy the old path.
@@ -303,9 +296,7 @@ Maybe<Url> Url::tryParseRelative(StringPtr text) const {
       // New path is relative, so start from the old path, dropping everything after the last
       // slash.
       auto slice = this->path.slice(0, this->path.size() - (this->hasTrailingSlash ? 0 : 1));
-      for (auto& part: slice) {
-        path.add(kj::str(part));
-      }
+      result.path = KJ_MAP(part, slice) { return kj::str(part); };
       result.hasTrailingSlash = true;
     }
 
@@ -313,22 +304,20 @@ Maybe<Url> Url::tryParseRelative(StringPtr text) const {
       auto part = split(text, END_PATH_PART);
       if (part.size() == 2 && part[0] == '.' && part[1] == '.') {
         if (path.size() != 0) {
-          path.removeLast();
+          result.path.removeLast();
         }
         result.hasTrailingSlash = true;
       } else if (part.size() == 0 || (part.size() == 1 && part[0] == '.')) {
         // Collapse consecutive slashes and "/./".
         result.hasTrailingSlash = true;
       } else {
-        path.add(percentDecode(part, err));
+        result.path.add(percentDecode(part, err));
         result.hasTrailingSlash = false;
       }
 
       if (!text.startsWith("/")) break;
       text = text.slice(1);
     }
-
-    result.path = path.releaseAsArray();
   } else if (!hadNewAuthority) {
     // copy path
     result.path = KJ_MAP(part, this->path) { return kj::str(part); };
@@ -336,20 +325,18 @@ Maybe<Url> Url::tryParseRelative(StringPtr text) const {
   }
 
   if (text.startsWith("?")) {
-    Vector<QueryParam> params;
     do {
       text = text.slice(1);
       auto part = split(text, END_QUERY_PART);
 
       if (part.size() > 0) {
         KJ_IF_MAYBE(key, trySplit(part, '=')) {
-          params.add(QueryParam { percentDecode(*key, err), percentDecode(part, err) });
+          result.query.add(QueryParam { percentDecode(*key, err), percentDecode(part, err) });
         } else {
-          params.add(QueryParam { percentDecode(part, err), nullptr });
+          result.query.add(QueryParam { percentDecode(part, err), nullptr });
         }
       }
     } while (text.startsWith("&"));
-    result.query = params.releaseAsArray();
   } else if (!hadNewAuthority && !hadNewPath) {
     // copy query
     result.query = KJ_MAP(param, this->query) {
@@ -404,6 +391,11 @@ String Url::toString(Context context) const {
   }
 
   for (auto& pathPart: path) {
+    // Protect against path injection.
+    KJ_REQUIRE(pathPart != "" && pathPart != "." && pathPart != "..",
+               "invalid name in URL path", *this) {
+      continue;
+    }
     chars.add('/');
     chars.addAll(encodeUriComponent(pathPart));
   }

c++/src/kj/compat/url.h

@@ -23,6 +23,7 @@
 #define KJ_COMPAT_URL_H_
 
 #include <kj/string.h>
+#include <kj/vector.h>
 #include <inttypes.h>
 
 namespace kj {
@@ -48,16 +49,20 @@ struct Url {
   // network address parsing functions already accept addresses containing port numbers, and
   // because most web standards don't actually want to separate host and port.
 
-  Array<String> path;
+  Vector<String> path;
   bool hasTrailingSlash = false;
   // Path, split on '/' characters. Note that the individual components of `path` could contain
   // '/' characters if they were percent-encoded in the original URL.
+  //
+  // No component of the path is allowed to be "", ".", nor ".."; if such components are present,
+  // toString() will throw. Note that parse() and parseRelative() automatically resolve such
+  // components.
 
   struct QueryParam {
     String name;
     String value;
   };
-  Array<QueryParam> query;
+  Vector<QueryParam> query;
   // Query, e.g. from "?key=value&key2=value2". If a component of the query contains no '=' sign,
   // it will be parsed as a key with an empty value.
 
@@ -71,12 +76,14 @@ struct Url {
   ~Url() noexcept(false);
   Url& operator=(Url&&) = default;
 
-  inline Url(String&& scheme, Maybe<UserInfo>&& userInfo, String&& host, Array<String>&& path,
-             bool hasTrailingSlash, Array<QueryParam>&& query, Maybe<String>&& fragment)
+#if __cplusplus < 201402L
+  inline Url(String&& scheme, Maybe<UserInfo>&& userInfo, String&& host, Vector<String>&& path,
+             bool hasTrailingSlash, Vector<QueryParam>&& query, Maybe<String>&& fragment)
       : scheme(kj::mv(scheme)), userInfo(kj::mv(userInfo)), host(kj::mv(host)), path(kj::mv(path)),
         hasTrailingSlash(hasTrailingSlash), query(kj::mv(query)), fragment(kj::mv(fragment)) {}
   // TODO(cleanup): This constructor is only here to support brace initialization in C++11. It
   //   should be removed once we upgrade to C++14.
+#endif
 
   Url clone() const;
 

c++/src/kj/vector.h

@@ -43,6 +43,7 @@ class Vector {
 public:
   inline Vector() = default;
   inline explicit Vector(size_t capacity): builder(heapArrayBuilder<T>(capacity)) {}
+  inline Vector(Array<T>&& array): builder(kj::mv(array)) {}
 
   inline operator ArrayPtr<T>() { return builder; }
   inline operator ArrayPtr<const T>() const { return builder; }
@@ -71,6 +72,18 @@ public:
     return builder.finish();
   }
 
+  template <typename U>
+  inline bool operator==(const U& other) const { return asPtr() == other; }
+  template <typename U>
+  inline bool operator!=(const U& other) const { return asPtr() != other; }
+
+  inline ArrayPtr<T> slice(size_t start, size_t end) {
+    return asPtr().slice(start, end);
+  }
+  inline ArrayPtr<const T> slice(size_t start, size_t end) const {
+    return asPtr().slice(start, end);
+  }
+
   template <typename... Params>
   inline T& add(Params&&... params) {
     if (builder.isFull()) grow();