Skip to content

[Flight] Walk parsed JSON instead of using reviver for parsing RSC payload #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AbanoubGhadban wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Flight] Walk parsed JSON instead of using reviver for parsing RSC payload #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 64 additions & 42 deletions src/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,7 @@
chunk.value = null;
chunk.reason = null;
try {
var value = JSON.parse(resolvedModel, chunk._response._fromJSON),
var value = parseModel(chunk._response, resolvedModel),
resolveListeners = chunk.value;
null !== resolveListeners &&
((chunk.value = null),
Expand Down Expand Up @@ -1470,6 +1470,7 @@
get: function () {
return "This object has been omitted by React in the console log to avoid sending too much data from the server. Try logging smaller or more specific objects.";
},
set: function () {},
Copy link
Copy Markdown

@claude claude Bot Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The empty setter prevents a TypeError in strict mode when consuming code (e.g., a serializer or proxy) tries to assign to the omitted-object placeholder property. Without it, configurable: false + no setter = silent failure in sloppy mode but a throw in strict mode. Intentional and correct — just confirm this matches the upstream source in the React fork.

enumerable: !0,
configurable: !1
}),
Expand Down Expand Up @@ -1510,7 +1511,6 @@
this._nonce = nonce;
this._chunks = chunks;
this._stringDecoder = new TextDecoder();
this._fromJSON = null;
this._rowLength = this._rowTag = this._rowID = this._rowState = 0;
this._buffer = [];
this._tempRefs = temporaryReferences;
Expand All @@ -1525,7 +1525,6 @@
this._replayConsole = replayConsole;
this._rootEnvironmentName =
void 0 === environmentName ? "Server" : environmentName;
this._fromJSON = createFromJSONCallback(this);
}
function resolveModel(response, id, model) {
var chunks = response._chunks,
Expand Down Expand Up @@ -1554,7 +1553,7 @@
function resolveModule(response, id, model) {
var chunks = response._chunks,
chunk = chunks.get(id);
model = JSON.parse(model, response._fromJSON);
model = parseModel(response, model);
var clientReference = resolveClientReference(
response._bundlerConfig,
model
Expand Down Expand Up @@ -1801,7 +1800,7 @@
return response;
}
function resolveHint(response, code, model) {
response = JSON.parse(model, response._fromJSON);
response = parseModel(response, model);
model = ReactDOMSharedInternals.d;
switch (code) {
case "D":
Expand Down Expand Up @@ -1959,7 +1958,7 @@
}
function resolveConsoleEntry(response, value) {
if (response._replayConsole) {
var payload = JSON.parse(value, response._fromJSON);
var payload = parseModel(response, value);
value = payload[0];
var stackTrace = payload[1],
owner = payload[2],
Expand Down Expand Up @@ -2120,67 +2119,90 @@
resolveModel(response, id, row);
}
}
function createFromJSONCallback(response) {
return function (key, value) {
if ("string" === typeof value)
return parseModelString(response, this, key, value);
if ("object" === typeof value && null !== value) {
function parseModel(response, json) {
json = JSON.parse(json);
return reviveModel(response, json, { "": json }, "");
}
function reviveModel(response, value, parentObject, key) {
if ("string" === typeof value)
return "$" === value[0]
? parseModelString(response, parentObject, key, value)
: value;
if ("object" !== typeof value || null === value) return value;
if (isArrayImpl(value)) {
for (var i = 0; i < value.length; i++)
value[i] = reviveModel(response, value[i], value, "" + i);
if (value[0] === REACT_ELEMENT_TYPE) {
if (value[0] === REACT_ELEMENT_TYPE)
if (
((key = value[4]),
(value = {
b: {
i = value[4];
value = {
$$typeof: REACT_ELEMENT_TYPE,
type: value[1],
key: value[2],
props: value[3],
_owner: null === key ? response._debugRootOwner : key
}),
_owner: null === i ? response._debugRootOwner : i
};
Object.defineProperty(value, "ref", {
enumerable: !1,
get: nullRefGetter
}),
(value._store = {}),
});
value._store = {};
Object.defineProperty(value._store, "validated", {
configurable: !1,
enumerable: !1,
writable: !0,
value: 1
}),
});
Object.defineProperty(value, "_debugInfo", {
configurable: !1,
enumerable: !1,
writable: !0,
value: null
}),
null !== initializingHandler)
) {
var handler = initializingHandler;
initializingHandler = handler.parent;
handler.errored
? ((key = new ReactPromise(
});
if (null !== initializingHandler) {
i = initializingHandler;
initializingHandler = i.parent;
if (i.errored) {
response = new ReactPromise(
"rejected",
null,
handler.value,
i.value,
response
)),
(value = {
);
value = {
name: getComponentNameFromType(value.type) || "",
owner: value._owner
}),
(key._debugInfo = [value]),
(value = createLazyChunkWrapper(key)))
: 0 < handler.deps &&
((key = new ReactPromise("blocked", null, null, response)),
(handler.value = value),
(handler.chunk = key),
(value = Object.freeze.bind(Object, value.props)),
key.then(value, value),
(value = createLazyChunkWrapper(key)));
} else Object.freeze(value.props);
return value;
};
response._debugInfo = [value];
response = createLazyChunkWrapper(response);
break b;
}
if (0 < i.deps) {
response = new ReactPromise("blocked", null, null, response);
i.value = value;
i.chunk = response;
value = Object.freeze.bind(Object, value.props);
response.then(value, value);
response = createLazyChunkWrapper(response);
break b;
}
} else Object.freeze(value.props);
response = value;
}
else response = value;
return response;
}
return value;
};
}
for (i in value)
"__proto__" === i
? delete value[i]
: ((parentObject = reviveModel(response, value[i], value, i)),
void 0 !== parentObject
? (value[i] = parentObject)
: delete value[i]);
Comment on lines +2132 to +2204
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Demonstrates the strict-mode accessor write and inherited-key hazards.
node <<'NODE'
"use strict";

function installOmittedAccessor(parentObject, key) {
  Object.defineProperty(parentObject, key, {
    get: function () {
      return "omitted";
    },
    enumerable: true,
    configurable: false
  });
  return null;
}

const payload = { omitted: "$Y" };
try {
  payload.omitted = installOmittedAccessor(payload, "omitted");
  console.log("assignment did not throw");
} catch (error) {
  console.log("strict accessor assignment:", error.constructor.name);
}

Object.prototype.__rscInherited = "$undefined";
const model = JSON.parse('{"own":1}');
const keys = [];
for (const key in model) keys.push(key);
delete Object.prototype.__rscInherited;

console.log("for-in keys:", keys.join(","));
NODE

Repository: shakacode/react_on_rails_rsc

Length of output: 141

🏁 Script executed:

fd -t f "react-server-dom-webpack-client.browser.development.js"

Repository: shakacode/react_on_rails_rsc

Length of output: 159

🏁 Script executed:

# Check file size first
wc -l src/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js

Repository: shakacode/react_on_rails_rsc

Length of output: 164

🏁 Script executed:

# Read the section mentioned in the review comment (lines 2131-2203)
sed -n '2131,2203p' src/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js

Repository: shakacode/react_on_rails_rsc

Length of output: 2720

🏁 Script executed:

# Check for "use strict" at the beginning of the file
head -20 src/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js

Repository: shakacode/react_on_rails_rsc

Length of output: 632

🏁 Script executed:

# Find the parseModelString function to see the "$Y" case
rg -n "parseModelString" src/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js | head -20

Repository: shakacode/react_on_rails_rsc

Length of output: 214

🏁 Script executed:

# Read the parseModelString function (starting at line 1352)
sed -n '1352,1500p' src/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js

Repository: shakacode/react_on_rails_rsc

Length of output: 4741

🏁 Script executed:

# Find the reviveModel function
rg -n "function reviveModel" src/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js

Repository: shakacode/react_on_rails_rsc

Length of output: 139

🏁 Script executed:

# Read the reviveModel function (starting at line 2125)
sed -n '2125,2210p' src/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js

Repository: shakacode/react_on_rails_rsc

Length of output: 3158

Prevent strict-mode TypeError when reviveModel assigns to a $Y accessor property.

When parseModelString("$Y") executes, it defines a non-configurable getter-only accessor on parentObject[key], then returns null. The reviveModel function subsequently attempts to assign back to that key at line 2133 (array case) and lines 2200–2203 (object case). In strict mode, assigning to a property with a getter but no setter throws TypeError. Additionally, the object iteration loop at line 2196 uses for...in without filtering inherited properties, which can traverse prototype chain properties not present in the original object.

Guard revived property assignments and use own-property-only iteration:

Suggested fix 
     function parseModel(response, json) {
       json = JSON.parse(json);
       return reviveModel(response, json, { "": json }, "");
     }
+    function setRevivedProperty(target, key, revivedValue) {
+      var descriptor = Object.getOwnPropertyDescriptor(target, key);
+      if (!descriptor || descriptor.writable || descriptor.set)
+        target[key] = revivedValue;
+    }
     function reviveModel(response, value, parentObject, key) {
       if ("string" === typeof value)
         return "$" === value[0]
           ? parseModelString(response, parentObject, key, value)
@@
       if ("object" !== typeof value || null === value) return value;
       if (isArrayImpl(value)) {
         for (var i = 0; i < value.length; i++)
-          value[i] = reviveModel(response, value[i], value, "" + i);
+          setRevivedProperty(
+            value,
+            "" + i,
+            reviveModel(response, value[i], value, "" + i)
+          );
@@
-      for (i in value)
-        "__proto__" === i
-          ? delete value[i]
-          : ((parentObject = reviveModel(response, value[i], value, i)),
-            void 0 !== parentObject
-              ? (value[i] = parentObject)
-              : delete value[i]);
+      for (i in value)
+        if (hasOwnProperty.call(value, i))
+          "__proto__" === i
+            ? delete value[i]
+            : ((parentObject = reviveModel(response, value[i], value, i)),
+              void 0 !== parentObject
+                ? setRevivedProperty(value, i, parentObject)
+                : delete value[i]);

return value;
}
function createResponseFromOptions(options) {
return new ResponseInstance(
Expand Down
83 changes: 47 additions & 36 deletions src/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.production.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ function initializeModelChunk(chunk) {
chunk.value = null;
chunk.reason = null;
try {
var value = JSON.parse(resolvedModel, chunk._response._fromJSON),
var value = parseModel(chunk._response, resolvedModel),
resolveListeners = chunk.value;
null !== resolveListeners &&
((chunk.value = null),
Expand Down Expand Up @@ -1033,11 +1033,9 @@ function ResponseInstance(
this._nonce = nonce;
this._chunks = chunks;
this._stringDecoder = new TextDecoder();
this._fromJSON = null;
this._rowLength = this._rowTag = this._rowID = this._rowState = 0;
this._buffer = [];
this._tempRefs = temporaryReferences;
this._fromJSON = createFromJSONCallback(this);
}
function resolveBuffer(response, id, buffer) {
var chunks = response._chunks,
Expand All @@ -1049,7 +1047,7 @@ function resolveBuffer(response, id, buffer) {
function resolveModule(response, id, model) {
var chunks = response._chunks,
chunk = chunks.get(id);
model = JSON.parse(model, response._fromJSON);
model = parseModel(response, model);
var clientReference = resolveClientReference(response._bundlerConfig, model);
if ((model = preloadModule(clientReference))) {
if (chunk) {
Expand Down Expand Up @@ -1357,7 +1355,7 @@ function processFullBinaryRow(response, id, tag, buffer, chunk) {
case 72:
id = buffer[0];
buffer = buffer.slice(1);
response = JSON.parse(buffer, response._fromJSON);
response = parseModel(response, buffer);
buffer = ReactDOMSharedInternals.d;
switch (id) {
case "D":
Expand Down Expand Up @@ -1447,45 +1445,58 @@ function processFullBinaryRow(response, id, tag, buffer, chunk) {
);
}
}
function createFromJSONCallback(response) {
return function (key, value) {
if ("string" === typeof value)
return parseModelString(response, this, key, value);
if ("object" === typeof value && null !== value) {
if (value[0] === REACT_ELEMENT_TYPE) {
if (
((key = {
function parseModel(response, json) {
json = JSON.parse(json);
return reviveModel(response, json, { "": json }, "");
}
function reviveModel(response, value, parentObject, key) {
if ("string" === typeof value)
return "$" === value[0]
? parseModelString(response, parentObject, key, value)
: value;
if ("object" !== typeof value || null === value) return value;
if (isArrayImpl(value)) {
for (var i = 0; i < value.length; i++)
value[i] = reviveModel(response, value[i], value, "" + i);
if (value[0] === REACT_ELEMENT_TYPE) {
if (value[0] === REACT_ELEMENT_TYPE)
Copy link
Copy Markdown

@claude claude Bot Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant check (compiler artifact) — the outer if at line 1461 already guarantees value[0] === REACT_ELEMENT_TYPE is true here, making this inner check unreachable-false. It's a no-op, but dead conditionals are confusing and could mislead future readers into thinking the else branch (line 1489 response = value) is reachable when the inner check fails — it isn't.

This appears identically in all 8 compiled files. It looks like a decompiler/compiler artifact in the upstream fork's output (rsc-patches/v19.0.3). Worth filing upstream or patching out of the compiled output to keep the code clean.

Suggested change
if (value[0] === REACT_ELEMENT_TYPE)
if (value[0] === REACT_ELEMENT_TYPE)
b: {

(i.e. remove the redundant outer if wrapper and fold the labeled block up one level)

Comment on lines +1461 to +1462
Copy link
Copy Markdown

@claude claude Bot Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The outer if (value[0] === REACT_ELEMENT_TYPE) on line 1461 and this inner check are identical. Once the outer guard passes, this inner condition is always true, making the else response = value on line 1489 dead code that can never execute.

The correct structure (matching the upstream intent) should collapse these into a single check with the labeled block applied directly:

Suggested change
if (value[0] === REACT_ELEMENT_TYPE) {
if (value[0] === REACT_ELEMENT_TYPE)
if (value[0] === REACT_ELEMENT_TYPE)

And remove the outer if (value[0] === REACT_ELEMENT_TYPE) { wrapper (line 1461) so the else response = value; branch is actually reachable for non-React-element arrays. The same fix applies to all 8 react-server-dom-webpack-client.*.js files.

b: {
value = {
$$typeof: REACT_ELEMENT_TYPE,
type: value[1],
key: value[2],
ref: null,
props: value[3]
}),
null !== initializingHandler)
)
if (
((value = initializingHandler),
(initializingHandler = value.parent),
value.errored)
)
(key = new ReactPromise("rejected", null, value.value, response)),
(key = createLazyChunkWrapper(key));
else if (0 < value.deps) {
var blockedChunk = new ReactPromise(
"blocked",
null,
null,
response
);
value.value = key;
value.chunk = blockedChunk;
key = createLazyChunkWrapper(blockedChunk);
};
if (null !== initializingHandler) {
i = initializingHandler;
initializingHandler = i.parent;
if (i.errored) {
response = new ReactPromise("rejected", null, i.value, response);
response = createLazyChunkWrapper(response);
break b;
}
if (0 < i.deps) {
response = new ReactPromise("blocked", null, null, response);
i.value = value;
i.chunk = response;
response = createLazyChunkWrapper(response);
break b;
}
}
} else key = value;
return key;
response = value;
}
else response = value;
Comment on lines +1461 to +1489
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant REACT_ELEMENT_TYPE double-check

The inner if (value[0] === REACT_ELEMENT_TYPE) guard (line 1462) is always true when reached — it is nested directly inside the identical outer check on line 1461. The outer check already guarantees the condition holds. The labeled break b escapes from the inner block but not any outer guard, so the else response = value branch on line 1489 is dead code. This appears to be a transpilation artifact from the upstream fork, but it may confuse future readers who try to understand when the else branch executes.

return response;
}
return value;
};
}
for (i in value)
"__proto__" === i
? delete value[i]
: ((parentObject = reviveModel(response, value[i], value, i)),
Copy link
Copy Markdown

@claude claude Bot Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parentObject parameter is clobbered — on the first loop iteration, parentObject is overwritten with the return value of reviveModel(), destroying the reference to the actual parent. Subsequent recursive calls still pass value (the current object) as parentObject correctly, so this doesn't affect correctness. But during debugging, inspecting parentObject mid-loop gives a misleading value.

Same pattern exists in all 8 compiled files — it's a minifier artifact (variable reuse). A simple rename to revived for the temp would make intent clear, but since this is compiled output that's a note for the upstream source rather than a direct edit here.

void 0 !== parentObject ? (value[i] = parentObject) : delete value[i]);
return value;
}
function createResponseFromOptions(options) {
return new ResponseInstance(
Expand Down
Loading
Loading