fix: implement validation for Dictionary names and keys

This commit updates our Dictionary implementation to ensure we throw nice error messages when given invalid dictionary names or keys, such as a name for a non-existant dictionary or a key that is too long.

Author: Jake Champion

c-dependencies/js-compute-runtime/builtins/dictionary.cpp

@@ -11,14 +11,32 @@ DictionaryHandle Dictionary::dictionary_handle(JSObject *obj) {
 bool Dictionary::get(JSContext *cx, unsigned argc, JS::Value *vp) {
   METHOD_HEADER(1)
 
+  JS::HandleValue name_arg = args.get(0);
+
   size_t name_len;
-  JS::UniqueChars name = encode(cx, args[0], &name_len);
+  // Convert into a String following https://tc39.es/ecma262/#sec-tostring
+  JS::UniqueChars name_chars = encode(cx, name_arg, &name_len);
+  if (!name_chars) {
+    return false;
+  }
+
+  // If the converted string has a length of 0 then we throw an Error
+  // because Dictionary keys have to be at-least 1 character.
+  if (name_len == 0) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_DICTIONARY_KEY_EMPTY);
+    return false;
+  }
+  // key has to be less than 256
+  if (name_len > 255) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_DICTIONARY_KEY_TOO_LONG);
+    return false;
+  }
 
   OwnedHostCallBuffer buffer;
   size_t nwritten = 0;
-  auto status = convert_to_fastly_status(xqd_dictionary_get(Dictionary::dictionary_handle(self),
-                                                            name.get(), name_len, buffer.get(),
-                                                            DICTIONARY_ENTRY_MAX_LEN, &nwritten));
+  auto status = convert_to_fastly_status(
+      xqd_dictionary_get(Dictionary::dictionary_handle(self), name_chars.get(), name_len,
+                         buffer.get(), DICTIONARY_ENTRY_MAX_LEN, &nwritten));
   // FastlyStatus::none indicates the key wasn't found, so we return null.
   if (status == FastlyStatus::None) {
     args.rval().setNull();
@@ -45,17 +63,65 @@ bool Dictionary::constructor(JSContext *cx, unsigned argc, JS::Value *vp) {
   REQUEST_HANDLER_ONLY("The Dictionary builtin");
   CTOR_HEADER("Dictionary", 1);
 
+  JS::HandleValue name_arg = args.get(0);
+
   size_t name_len;
-  JS::UniqueChars name = encode(cx, args[0], &name_len);
+  // Convert into a String following https://tc39.es/ecma262/#sec-tostring
+  JS::UniqueChars name_chars = encode(cx, name_arg, &name_len);
+  if (!name_chars) {
+    return false;
+  }
+
+  // If the converted string has a length of 0 then we throw an Error
+  // because Dictionary names have to be at-least 1 character.
+  if (name_len == 0) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_DICTIONARY_NAME_EMPTY);
+    return false;
+  }
+
+  // If the converted string has a length of more than 255 then we throw an Error
+  // because Dictionary names have to be less than 255 characters.
+  if (name_len > 255) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_DICTIONARY_NAME_TOO_LONG);
+    return false;
+  }
+
+  std::string_view name(name_chars.get(), name_len);
+
+  // Name must start with ascii alphabetical and contain only ascii alphanumeric, underscore, and
+  // whitespace
+  if (!std::isalpha(name.front())) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                              JSMSG_DICTIONARY_NAME_START_WITH_ASCII_ALPHA);
+    return false;
+  }
+
+  auto is_valid_name = std::all_of(std::next(name.begin(), 1), name.end(), [&](auto character) {
+    return std::isalnum(character) || character == '_' || character == ' ';
+  });
+
+  if (!is_valid_name) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                              JSMSG_DICTIONARY_NAME_CONTAINS_INVALID_CHARACTER);
+    return false;
+  }
   JS::RootedObject dictionary(cx, JS_NewObjectForConstructor(cx, &class_, args));
   DictionaryHandle dict_handle = {INVALID_HANDLE};
-  if (!HANDLE_RESULT(cx, xqd_dictionary_open(name.get(), name_len, &dict_handle)))
+  auto status = convert_to_fastly_status(xqd_dictionary_open(name.data(), name_len, &dict_handle));
+  if (status == FastlyStatus::BadF) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_DICTIONARY_DOES_NOT_EXIST,
+                              name.data());
     return false;
+  }
+  if (!HANDLE_RESULT(cx, status)) {
+    return false;
+  }
 
   JS::SetReservedSlot(dictionary, Dictionary::Slots::Handle,
                       JS::Int32Value((int)dict_handle.handle));
-  if (!dictionary)
+  if (!dictionary) {
     return false;
+  }
   args.rval().setObject(*dictionary);
   return true;
 }

c-dependencies/js-compute-runtime/error-numbers.msg

@@ -45,6 +45,13 @@ MSG_DEF(JSMSG_INCOMPATIBLE_INSTANCE,                           2, JSEXN_TYPEERR,
 MSG_DEF(JSMSG_INVALID_BUFFER_ARG,                              2, JSEXN_TYPEERR, "{0} must be of type ArrayBuffer or ArrayBufferView but got \"{1}\"")
 MSG_DEF(JSMSG_INVALID_COMPRESSION_FORMAT,                      1, JSEXN_TYPEERR, "'format' has to be \"deflate\", \"deflate-raw\", or \"gzip\", but got \"{0}\"")
 MSG_DEF(JSMSG_DECOMPRESSING_ERROR,                             0, JSEXN_TYPEERR, "DecompressionStream transform: error decompressing chunk")
+MSG_DEF(JSMSG_DICTIONARY_DOES_NOT_EXIST,                       1, JSEXN_TYPEERR, "Dictionary constructor: No Dictionary named '{0}' exists")
+MSG_DEF(JSMSG_DICTIONARY_KEY_EMPTY,                            0, JSEXN_TYPEERR, "Dictionary key can not be an empty string")
+MSG_DEF(JSMSG_DICTIONARY_KEY_TOO_LONG,                         0, JSEXN_TYPEERR, "Dictionary key can not be more than 255 characters")
+MSG_DEF(JSMSG_DICTIONARY_NAME_CONTAINS_INVALID_CHARACTER,      0, JSEXN_TYPEERR, "Dictionary constructor: name can contain only ascii alphanumeric characters, underscores, and ascii whitespace")
+MSG_DEF(JSMSG_DICTIONARY_NAME_EMPTY,                           0, JSEXN_TYPEERR, "Dictionary constructor: name can not be an empty string")
+MSG_DEF(JSMSG_DICTIONARY_NAME_START_WITH_ASCII_ALPHA,          0, JSEXN_TYPEERR, "Dictionary constructor: name must start with an ascii alpabetical character")
+MSG_DEF(JSMSG_DICTIONARY_NAME_TOO_LONG,                        0, JSEXN_TYPEERR, "Dictionary constructor: name can not be more than 255 characters")
 MSG_DEF(JSMSG_OBJECT_STORE_NAME_EMPTY,                         0, JSEXN_TYPEERR, "ObjectStore constructor: name can not be an empty string")
 MSG_DEF(JSMSG_OBJECT_STORE_NAME_TOO_LONG,                      0, JSEXN_TYPEERR, "ObjectStore constructor: name can not be more than 255 characters")
 MSG_DEF(JSMSG_OBJECT_STORE_NAME_NO_CONTROL_CHARACTERS,         0, JSEXN_TYPEERR, "ObjectStore constructor: name can not contain control characters (\\u0000-\\u001F)")