Improve handling of GC managed array buffers (#61)

Specifically, some of the `noGC` guards had issues that could lead to panics during exception throwing, and some of the loops over multiple buffers where overly complex.

builtins/web/base64.cpp

@@ -20,35 +20,33 @@ JS::Result<std::string> convertJSValueToByteString(JSContext *cx, JS::Handle<JS:
   // Conversion from JavaScript string to ByteString is only valid if all
   // characters < 256. This is always the case for Latin1 strings.
   size_t length;
+  UniqueChars result = nullptr;
   if (!JS::StringHasLatin1Chars(s)) {
-    // Creating an exception can GC, so we first scan the string for bad chars
-    // and report the error outside the AutoCheckCannotGC scope.
-    bool foundBadChar = false;
-    {
-      JS::AutoCheckCannotGC nogc;
-      const char16_t *chars = JS_GetTwoByteStringCharsAndLength(cx, nogc, s, &length);
-      if (!chars) {
+    JS::AutoCheckCannotGC nogc(cx);
+    const char16_t *chars = JS_GetTwoByteStringCharsAndLength(cx, nogc, s, &length);
+    if (!chars) {
+      // Reset the nogc guard, since otherwise we can't throw errors.
+      nogc.reset();
+      JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr, JSMSG_INVALID_CHARACTER_ERROR);
+      return JS::Result<std::string>(JS::Error());
+    }
+
+    for (size_t i = 0; i < length; i++) {
+      if (chars[i] > 255) {
+        // Reset the nogc guard, since otherwise we can't throw errors.
+        nogc.reset();
         JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr, JSMSG_INVALID_CHARACTER_ERROR);
         return JS::Result<std::string>(JS::Error());
       }
-
-      for (size_t i = 0; i < length; i++) {
-        if (chars[i] > 255) {
-          foundBadChar = true;
-          break;
-        }
-      }
-    }
-
-    if (foundBadChar) {
-      JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr, JSMSG_INVALID_CHARACTER_ERROR);
-      return JS::Result<std::string>(JS::Error());
     }
+    auto latin1_z =
+        JS::LossyTwoByteCharsToNewLatin1CharsZ(cx, chars, length);
+    result = UniqueChars(reinterpret_cast<char*>(latin1_z.get()));
   } else {
     length = JS::GetStringLength(s);
+    result = JS_EncodeStringToLatin1(cx, s);
   }
 
-  UniqueChars result = JS_EncodeStringToLatin1(cx, s);
   if (!result) {
     return JS::Result<std::string>(JS::Error());
   }

builtins/web/crypto/crypto.cpp

@@ -54,6 +54,7 @@ bool Crypto::get_random_values(JSContext *cx, unsigned argc, JS::Value *vp) {
 
   auto res = host_api::Random::get_bytes(byte_length);
   if (auto *err = res.to_err()) {
+    noGC.reset();
     HANDLE_ERROR(cx, *err);
     return false;
   }

builtins/web/fetch/request-response.cpp

@@ -621,71 +621,44 @@ bool RequestOrResponse::content_stream_read_then_handler(JSContext *cx, JS::Hand
     if (!JS::GetArrayLength(cx, contents, &contentsLength)) {
       return false;
     }
-    // TODO(performance): investigate whether we can infer the size directly from `contents`
-    size_t buf_size = HANDLE_READ_CHUNK_SIZE;
-    // TODO(performance): make use of malloc slack.
-    // https://github.com/fastly/js-compute-runtime/issues/217
-    size_t offset = 0;
-    // In this loop we are finding the length of each entry in `contents` and resizing the `buf`
-    // until it is large enough to fit all the entries in `contents`
-    for (uint32_t index = 0; index < contentsLength; index++) {
-      JS::RootedValue val(cx);
+
+    size_t total_length = 0;
+    RootedValue val(cx);
+
+    for (size_t index = 0; index < contentsLength; index++) {
       if (!JS_GetElement(cx, contents, index, &val)) {
         return false;
       }
-      {
-        JS::AutoCheckCannotGC nogc;
-        MOZ_ASSERT(val.isObject());
-        JSObject *array = &val.toObject();
-        MOZ_ASSERT(JS_IsUint8Array(array));
-        size_t length = JS_GetTypedArrayByteLength(array);
-        if (length) {
-          offset += length;
-          // if buf is not big enough to fit the next uint8array's bytes then resize
-          if (offset > buf_size) {
-            buf_size =
-                buf_size + (HANDLE_READ_CHUNK_SIZE * ((length / HANDLE_READ_CHUNK_SIZE) + 1));
-          }
-        }
-      }
+      JSObject *array = &val.toObject();
+      size_t length = JS_GetTypedArrayByteLength(array);
+      total_length += length;
     }
 
-    JS::UniqueChars buf{static_cast<char *>(JS_malloc(cx, buf_size + 1))};
+    JS::UniqueChars buf{static_cast<char *>(JS_malloc(cx, total_length))};
     if (!buf) {
       JS_ReportOutOfMemory(cx);
       return false;
     }
-    // reset the offset for the next loop
-    offset = 0;
+
+    size_t offset = 0;
     // In this loop we are inserting each entry in `contents` into `buf`
     for (uint32_t index = 0; index < contentsLength; index++) {
-      JS::RootedValue val(cx);
       if (!JS_GetElement(cx, contents, index, &val)) {
         return false;
       }
-      {
-        JS::AutoCheckCannotGC nogc;
-        MOZ_ASSERT(val.isObject());
-        JSObject *array = &val.toObject();
-        MOZ_ASSERT(JS_IsUint8Array(array));
-        bool is_shared;
-        size_t length = JS_GetTypedArrayByteLength(array);
-        if (length) {
-          static_assert(CHAR_BIT == 8, "Strange char");
-          auto bytes = reinterpret_cast<char *>(JS_GetUint8ArrayData(array, &is_shared, nogc));
-          memcpy(buf.get() + offset, bytes, length);
-          offset += length;
-        }
-      }
-    }
-    buf[offset] = '\0';
-#ifdef DEBUG
-    bool foundBodyParser;
-    if (!JS_HasElement(cx, catch_handler, 2, &foundBodyParser)) {
-      return false;
+      JSObject *array = &val.toObject();
+      bool is_shared;
+      size_t length = JS_GetTypedArrayByteLength(array);
+      JS::AutoCheckCannotGC nogc(cx);
+      auto bytes = reinterpret_cast<char *>(JS_GetUint8ArrayData(array, &is_shared, nogc));
+      memcpy(buf.get() + offset, bytes, length);
+      offset += length;
     }
+
+    mozilla::DebugOnly foundBodyParser = false;
+    MOZ_ASSERT(JS_HasElement(cx, catch_handler, 2, &foundBodyParser));
     MOZ_ASSERT(foundBodyParser);
-#endif
+
     // Now we can call parse_body on the result
     JS::RootedValue body_parser(cx);
     if (!JS_GetElement(cx, catch_handler, 2, &body_parser)) {

builtins/web/streams/compression-stream.cpp

@@ -129,7 +129,7 @@ bool deflate_chunk(JSContext *cx, JS::HandleObject self, JS::HandleValue chunk,
 
       {
         bool is_shared;
-        JS::AutoCheckCannotGC nogc;
+        JS::AutoCheckCannotGC nogc(cx);
         uint8_t *out_buffer = JS_GetUint8ArrayData(out_obj, &is_shared, nogc);
         memcpy(out_buffer, buffer, bytes);
       }