fix: copy the object-store key parameter into a native object to avoid pointing to a value which could get garbage collected

currently the value being used is a pointer to a JS String's UTF 8 representation and the JS String can become garbage collected, leading to the pointer no longer being valid

Author: Jake Champion

c-dependencies/js-compute-runtime/builtins/object-store.cpp

@@ -106,32 +106,42 @@ ObjectStoreHandle object_store_handle(JSObject *obj) {
 
 const unsigned ctor_length = 1;
 
-std::optional<char *> parse_and_validate_key(JSContext *cx, JS::HandleValue val, size_t *key_len) {
+std::optional<std::string> parse_and_validate_key(JSContext *cx, JS::HandleValue val) {
+  size_t key_len;
   // Convert the key argument into a String following https://tc39.es/ecma262/#sec-tostring
-  JS::UniqueChars key = encode(cx, val, key_len);
-  if (!key) {
+  JS::UniqueChars keyString = encode(cx, val, &key_len);
+  if (!keyString) {
     return std::nullopt;
   }
 
   // If the converted string has a length of 0 then we throw an Error
   // because ObjectStore Keys have to be at-least 1 character.
-  if (*key_len == 0) {
+  if (key_len == 0) {
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_OBJECT_STORE_KEY_EMPTY);
     return std::nullopt;
   }
 
   // If the converted string has a length of more than 1024 then we throw an Error
   // because ObjectStore Keys have to be less than 1025 characters.
-  if (*key_len > 1024) {
+  if (key_len > 1024) {
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_OBJECT_STORE_KEY_TOO_LONG);
     return std::nullopt;
   }
 
-  char *key_chars = key.get();
+  std::string key(keyString.get(), key_len);
+  auto key_chars = key.c_str();
 
   if (auto res = find_invalid_character_for_object_store_key(key_chars)) {
-    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                              JSMSG_OBJECT_STORE_KEY_INVALID_CHARACTER, *res);
+    if (*res == '\n') {
+      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                                JSMSG_OBJECT_STORE_KEY_INVALID_CHARACTER, "newline");
+    } else if (*res == '\r') {
+      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                                JSMSG_OBJECT_STORE_KEY_INVALID_CHARACTER, "carriage return");
+    } else {
+      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                                JSMSG_OBJECT_STORE_KEY_INVALID_CHARACTER, res);
+    }
     return std::nullopt;
   }
   auto acme_challenge = ".well-known/acme-challenge/";
@@ -145,7 +155,7 @@ std::optional<char *> parse_and_validate_key(JSContext *cx, JS::HandleValue val,
     return std::nullopt;
   }
 
-  return key_chars;
+  return key;
 }
 
 bool check_receiver(JSContext *cx, JS::HandleValue receiver, const char *method_name);
@@ -158,14 +168,14 @@ bool get(JSContext *cx, unsigned argc, JS::Value *vp) {
     return ReturnPromiseRejectedWithPendingError(cx, args);
   }
 
-  size_t key_len;
-  std::optional<char *> key_chars = parse_and_validate_key(cx, args.get(0), &key_len);
+  JS::RootedValue key(cx, args.get(0));
+  auto key_chars = parse_and_validate_key(cx, key);
   if (!key_chars) {
     return ReturnPromiseRejectedWithPendingError(cx, args);
   }
   BodyHandle body_handle = {INVALID_HANDLE};
-  int status =
-      fastly_object_store_get(object_store_handle(self), key_chars.value(), key_len, &body_handle);
+  int status = fastly_object_store_get(object_store_handle(self), key_chars.value().c_str(),
+                                       key_chars.value().length(), &body_handle);
   if (!HANDLE_RESULT(cx, status)) {
     return false;
   }
@@ -198,8 +208,8 @@ bool put(JSContext *cx, unsigned argc, JS::Value *vp) {
     return ReturnPromiseRejectedWithPendingError(cx, args);
   }
 
-  size_t key_len;
-  std::optional<char *> key_chars = parse_and_validate_key(cx, args.get(0), &key_len);
+  JS::RootedValue key(cx, args.get(0));
+  auto key_chars = parse_and_validate_key(cx, key);
   if (!key_chars) {
     return ReturnPromiseRejectedWithPendingError(cx, args);
   }
@@ -232,8 +242,8 @@ bool put(JSContext *cx, unsigned argc, JS::Value *vp) {
       JS::RootedObject source_owner(cx, builtins::NativeStreamSource::owner(stream_source));
       BodyHandle body = RequestOrResponse::body_handle(source_owner);
 
-      int status =
-          fastly_object_store_insert(object_store_handle(self), key_chars.value(), key_len, body);
+      int status = fastly_object_store_insert(object_store_handle(self), key_chars.value().c_str(),
+                                              key_chars.value().length(), body);
       if (!HANDLE_RESULT(cx, status)) {
         return ReturnPromiseRejectedWithPendingError(cx, args);
       }
@@ -308,8 +318,8 @@ bool put(JSContext *cx, unsigned argc, JS::Value *vp) {
       return ReturnPromiseRejectedWithPendingError(cx, args);
     }
 
-    int status = fastly_object_store_insert(object_store_handle(self), key_chars.value(), key_len,
-                                            body_handle);
+    int status = fastly_object_store_insert(object_store_handle(self), key_chars.value().c_str(),
+                                            key_chars.value().length(), body_handle);
     // Ensure that we throw an exception for all unexpected host errors.
     if (!HANDLE_RESULT(cx, status)) {
       return RejectPromiseWithPendingError(cx, result_promise);