fixup! url,buffer: implement URL.createObjectURL

Author: jasnell

doc/api/buffer.md

@@ -4959,11 +4959,11 @@ added: REPLACEME
 
 > Stability: 1 - Experimental
 
-* `id` {string} A `'blob:node:...` URL string returned by a prior call to
+* `id` {string} A `'blob:nodedata:...` URL string returned by a prior call to
   `URL.createObjectURL()`.
 * Returns: {Blob}
 
-Resolves a `'blob:node:...'` an associated {Blob} object registered using
+Resolves a `'blob:nodedata:...'` an associated {Blob} object registered using
 a prior call to `URL.createObjectURL()`.
 
 ### `buffer.transcode(source, fromEnc, toEnc)`

doc/api/url.md

@@ -618,8 +618,8 @@ added: REPLACEME
 * `blob` {Blob}
 * Returns: {string}
 
-Creates a `'blob:node:...'` URL string that represents the given {Blob} object
-and can be used to retrieve the `Blob` later.
+Creates a `'blob:nodedata:...'` URL string that represents the given {Blob}
+object and can be used to retrieve the `Blob` later.
 
 ```js
 const {
@@ -641,8 +641,7 @@ The data stored by the registered {Blob} will be retained in memory until
 
 `Blob` objects are registered within the current thread. If using Worker
 Threads, `Blob` objects registered within one Worker will not be available
-to other workers or the main thread. The `threadId` of the owning thread
-is encoded within the generated object URL.
+to other workers or the main thread.
 
 #### `URL.revokeObjectURL(id)`
 <!-- YAML
@@ -651,7 +650,7 @@ added: REPLACEME
 
 > Stability: 1 - Experimental
 
-* `id` {string} A `'blob:node:...` URL string returned by a prior call to
+* `id` {string} A `'blob:nodedata:...` URL string returned by a prior call to
   `URL.createObjectURL()`.
 
 Removes the stored {Blob} identified by the given ID.

lib/internal/blob.js

@@ -7,10 +7,11 @@ const {
   ObjectDefineProperty,
   PromiseResolve,
   PromiseReject,
-  PromisePrototypeFinally,
+  SafePromisePrototypeFinally,
   ReflectConstruct,
   RegExpPrototypeTest,
   StringPrototypeToLowerCase,
+  StringPrototypeSplit,
   Symbol,
   SymbolIterator,
   SymbolToStringTag,
@@ -21,7 +22,7 @@ const {
   createBlob: _createBlob,
   FixedSizeBlobCopyJob,
   getDataObject,
-} = internalBinding('buffer');
+} = internalBinding('blob');
 
 const { TextDecoder } = require('internal/encoding');
 
@@ -66,35 +67,29 @@ const disallowedTypeCharacters = /[^\u{0020}-\u{007E}]/u;
 
 let Buffer;
 let ReadableStream;
-let threadId;
 let URL;
 
+
+// Yes, lazy loading is annoying but because of circular
+// references between the url, internal/blob, and buffer
+// modules, lazy loading here makes sure that things work.
+
 function lazyURL(id) {
-  if (URL === undefined)
-    URL = require('url').URL;
+  URL ??= require('url').URL;
   return new URL(id);
 }
 
 function lazyBuffer() {
-  if (Buffer === undefined)
-    Buffer = require('buffer').Buffer;
+  Buffer ??= require('buffer').Buffer;
   return Buffer;
 }
 
 function lazyReadableStream(options) {
-  if (ReadableStream === undefined) {
-    ReadableStream =
-      require('internal/webstreams/readablestream').ReadableStream;
-  }
+  ReadableStream ??=
+    require('internal/webstreams/readablestream').ReadableStream;
   return new ReadableStream(options);
 }
 
-function lazyThreadId() {
-  if (threadId === undefined)
-    threadId = require('worker_threads').threadId;
-  return threadId;
-}
-
 function isBlob(object) {
   return object?.[kHandle] !== undefined;
 }
@@ -275,15 +270,14 @@ class Blob {
       resolve(ab);
     };
     this[kArrayBufferPromise] =
-    PromisePrototypeFinally(
+    SafePromisePrototypeFinally(
       promise,
       () => this[kArrayBufferPromise] = undefined);
 
     return this[kArrayBufferPromise];
   }
 
   /**
-   *
    * @returns {Promise<string>}
    */
   async text() {
@@ -334,13 +328,18 @@ function resolveObjectURL(url) {
   url = `${url}`;
   try {
     const parsed = new lazyURL(url);
+
+    const split = StringPrototypeSplit(parsed.pathname, ':');
+
+    if (split.length !== 2)
+      return;
+
     const {
       0: base,
-      1: threadId,
-      2: id,
-    } = parsed.pathname.split(':');
+      1: id,
+    } = split;
 
-    if (base !== 'node' || +threadId !== lazyThreadId())
+    if (base !== 'nodedata')
       return;
 
     const ret = getDataObject(id);

lib/internal/url.js

@@ -42,17 +42,20 @@ const {
 
 const { getConstructorOf, removeColors } = require('internal/util');
 const {
-  ERR_ARG_NOT_ITERABLE,
-  ERR_INVALID_ARG_TYPE,
-  ERR_INVALID_ARG_VALUE,
-  ERR_INVALID_FILE_URL_HOST,
-  ERR_INVALID_FILE_URL_PATH,
-  ERR_INVALID_THIS,
-  ERR_INVALID_TUPLE,
-  ERR_INVALID_URL,
-  ERR_INVALID_URL_SCHEME,
-  ERR_MISSING_ARGS
-} = require('internal/errors').codes;
+  codes: {
+    ERR_ARG_NOT_ITERABLE,
+    ERR_INVALID_ARG_TYPE,
+    ERR_INVALID_ARG_VALUE,
+    ERR_INVALID_FILE_URL_HOST,
+    ERR_INVALID_FILE_URL_PATH,
+    ERR_INVALID_THIS,
+    ERR_INVALID_TUPLE,
+    ERR_INVALID_URL,
+    ERR_INVALID_URL_SCHEME,
+    ERR_MISSING_ARGS,
+    ERR_NO_CRYPTO,
+  },
+} = require('internal/errors');
 const {
   CHAR_AMPERSAND,
   CHAR_BACKWARD_SLASH,
@@ -103,7 +106,7 @@ const {
 const {
   storeDataObject,
   revokeDataObject,
-} = internalBinding('buffer');
+} = internalBinding('blob');
 
 const context = Symbol('context');
 const cannotBeBase = Symbol('cannot-be-base');
@@ -115,26 +118,22 @@ const kFormat = Symbol('format');
 
 let blob;
 let cryptoRandom;
-let threadId;
 
 function lazyBlob() {
-  if (blob === undefined)
-    blob = require('internal/blob');
+  blob ??= require('internal/blob');
   return blob;
 }
 
 function lazyCryptoRandom() {
-  if (cryptoRandom === undefined)
-    cryptoRandom = require('internal/crypto/random');
+  try {
+    cryptoRandom ??= require('internal/crypto/random');
+  } catch {
+    // If Node.js built without crypto support, we'll fall
+    // through here and handle it later.
+  }
   return cryptoRandom;
 }
 
-function lazyThreadId() {
-  if (threadId === undefined)
-    threadId = require('worker_threads').threadId;
-  return threadId;
-}
-
 // https://tc39.github.io/ecma262/#sec-%iteratorprototype%-object
 const IteratorPrototype = ObjectGetPrototypeOf(
   ObjectGetPrototypeOf([][SymbolIterator]())
@@ -959,22 +958,29 @@ class URL {
   }
 
   static createObjectURL(obj) {
+    const cryptoRandom = lazyCryptoRandom();
+    if (cryptoRandom === undefined)
+      throw new ERR_NO_CRYPTO();
+
+    // Yes, lazy loading is annoying but because of circular
+    // references between the url, internal/blob, and buffer
+    // modules, lazy loading here makes sure that things work.
     const blob = lazyBlob();
     if (!blob.isBlob(obj))
       throw new ERR_INVALID_ARG_TYPE('obj', 'Blob', obj);
 
-    const id = lazyCryptoRandom().randomUUID();
+    const id = cryptoRandom.randomUUID();
 
     storeDataObject(id, obj[blob.kHandle], obj.size, obj.type);
 
-    return `blob:node:${lazyThreadId()}:${id}`;
+    return `blob:nodedata:${id}`;
   }
 
   static revokeObjectURL(url) {
     url = `${url}`;
     try {
       const parsed = new URL(url);
-      const { 2: id } = parsed.pathname.split(':');
+      const { 1: id } = StringPrototypeSplit(parsed.pathname, ':');
       if (id !== undefined)
         revokeDataObject(id);
     } catch {

src/env-inl.h

@@ -1182,25 +1182,6 @@ void Environment::set_process_exit_handler(
   process_exit_handler_ = std::move(handler);
 }
 
-void Environment::store_data_object(
-    const std::string& uuid,
-    const Environment::StoredDataObject& data_object) {
-  data_objects[uuid] = data_object;
-}
-
-void Environment::erase_data_object(const std::string& uuid) {
-  data_objects.erase(uuid);
-}
-
-Environment::StoredDataObject Environment::get_data_object(
-    const std::string& uuid) {
-  auto item = data_objects.find(uuid);
-  if (item == data_objects.end()) {
-    return Environment::StoredDataObject {};
-  }
-  return item->second;
-}
-
 #define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
 #define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
 #define VS(PropertyName, StringValue) V(v8::String, PropertyName)

src/env.cc

@@ -644,8 +644,6 @@ void Environment::CleanupHandles() {
          !handle_wrap_queue_.IsEmpty()) {
     uv_run(event_loop(), UV_RUN_ONCE);
   }
-
-  data_objects.clear();
 }
 
 void Environment::StartProfilerIdleNotifier() {

src/env.h

@@ -1389,20 +1389,6 @@ class Environment : public MemoryRetainer {
 
   inline int32_t stack_trace_limit() const { return 10; }
 
-  struct StoredDataObject {
-    BaseObjectPtr<BaseObject> data_object;
-    size_t length;
-    std::string type;
-  };
-
-  inline void store_data_object(
-      const std::string& uuid,
-      const StoredDataObject& object);
-
-  inline void erase_data_object(const std::string& uuid);
-
-  inline StoredDataObject get_data_object(const std::string& uuid);
-
 #if HAVE_INSPECTOR
   void set_coverage_connection(
       std::unique_ptr<profiler::V8CoverageConnection> connection);
@@ -1625,9 +1611,6 @@ class Environment : public MemoryRetainer {
   // a given pointer.
   std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>
       released_allocated_buffers_;
-
-  // Maintains stored Blobs used with the URL.createObjectURL() API
-  std::unordered_map<std::string, StoredDataObject> data_objects;
 };
 
 }  // namespace node

src/node_binding.cc

@@ -40,6 +40,7 @@
 // __attribute__((constructor)) like mechanism in GCC.
 #define NODE_BUILTIN_STANDARD_MODULES(V)                                       \
   V(async_wrap)                                                                \
+  V(blob)                                                                      \
   V(block_list)                                                                \
   V(buffer)                                                                    \
   V(cares_wrap)                                                                \