Skip to content

Commit 066f03d

Browse files
committed
fix(scratch-vm): repair broken shadow references from cursed inputs bug
Projects corrupted by the Blockly blockToDom serialization bug (878291) contain inputs with shadow references that point to nonexistent blocks. This causes a TypeError crash in moveBlock when a reporter is dragged out of such an input, and perpetuates the corruption through save/load. Three fixes: 1. Load-time repair in deserializeBlocks: after deserialization, scan all inputs for shadow references that point to block IDs not present in the project. Recreate the shadow block by finding a peer block of the same opcode with an intact shadow (peer lookup). Falls back to a text shadow if no peer is available. Only primitive shadow types are recreated — menu shadows are cleared to prevent crashes. 2. Defensive guard in moveBlock: when restoring a shadow after a block is disconnected, check that the shadow block actually exists before accessing it. If missing, clear the stale reference instead of crashing. 3. findPeerShadow helper: scans the blocks map for another block of the same opcode that has a working primitive shadow on the same input name, to use as a template for recreation.
1 parent 4ec921e commit 066f03d

4 files changed

Lines changed: 374 additions & 2 deletions

File tree

packages/scratch-vm/src/engine/blocks.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,8 +775,15 @@ class Blocks {
775775
// this input, or null out the input's block.
776776
const shadow = oldParent.inputs[e.oldInput].shadow;
777777
if (shadow && e.id !== shadow) {
778-
oldParent.inputs[e.oldInput].block = shadow;
779-
this._blocks[shadow].parent = oldParent.id;
778+
if (this._blocks[shadow]) {
779+
oldParent.inputs[e.oldInput].block = shadow;
780+
this._blocks[shadow].parent = oldParent.id;
781+
} else {
782+
// Shadow block is referenced but missing — clear
783+
// the stale reference rather than crashing.
784+
oldParent.inputs[e.oldInput].block = null;
785+
oldParent.inputs[e.oldInput].shadow = null;
786+
}
780787
this._blocks[e.id].parent = null;
781788
} else {
782789
oldParent.inputs[e.oldInput].block = null;

packages/scratch-vm/src/serialization/sb3.js

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,33 @@ const primitiveOpcodeInfoMap = {
9595
data_listcontents: [LIST_PRIMITIVE, 'LIST']
9696
};
9797

98+
/**
99+
* Find a working shadow block for the given (opcode, inputName) pair by
100+
* looking at other blocks of the same opcode in the project. Returns a
101+
* shallow copy of the shadow block suitable for use as a template, or
102+
* null if no peer has a working shadow for that input.
103+
* @param {object} blocks The full blocks map for the target.
104+
* @param {string} opcode The parent block's opcode.
105+
* @param {string} inputName The name of the input.
106+
* @returns {?object} A template shadow block, or null.
107+
*/
108+
const findPeerShadow = function (blocks, opcode, inputName) {
109+
for (const peerId in blocks) {
110+
if (!hasOwnProperty.call(blocks, peerId)) continue;
111+
const peer = blocks[peerId];
112+
if (Array.isArray(peer) || peer.opcode !== opcode) continue;
113+
const peerInput = peer.inputs[inputName];
114+
if (!peerInput || !peerInput.shadow) continue;
115+
const shadowBlock = blocks[peerInput.shadow];
116+
if (!shadowBlock || Array.isArray(shadowBlock)) continue;
117+
// Only use primitive shadow types (text, math_number, etc.) as
118+
// templates — menu shadows have state that can't be safely copied.
119+
if (!hasOwnProperty.call(primitiveOpcodeInfoMap, shadowBlock.opcode)) continue;
120+
return shadowBlock;
121+
}
122+
return null;
123+
};
124+
98125
/**
99126
* Serializes primitives described above into a more compact format
100127
* @param {object} block the block to serialize
@@ -885,6 +912,70 @@ const deserializeBlocks = function (blocks) {
885912
block.inputs = {};
886913
}
887914

915+
// Third pass: repair missing or broken shadow blocks. Shadows can be
916+
// missing in two ways:
917+
// 1. shadow is a stale ID pointing to a nonexistent block
918+
// 2. shadow is null when it shouldn't be (lost before save)
919+
// Both are leftovers from a serialization bug where shadow blocks were
920+
// dropped during save. Recreate the shadow by finding a peer block of
921+
// the same opcode with an intact shadow on the same input (peer lookup).
922+
// Fall back to a text shadow if no peer is available.
923+
for (const blockId in blocks) {
924+
if (!Object.prototype.hasOwnProperty.call(blocks, blockId)) continue;
925+
const block = blocks[blockId];
926+
if (Array.isArray(block)) continue;
927+
for (const inputName in block.inputs) {
928+
if (!Object.prototype.hasOwnProperty.call(block.inputs, inputName)) continue;
929+
const input = block.inputs[inputName];
930+
931+
const shadowIsBroken = input.shadow &&
932+
!Object.prototype.hasOwnProperty.call(blocks, input.shadow);
933+
const shadowIsMissing = !input.shadow && input.block &&
934+
Object.prototype.hasOwnProperty.call(blocks, input.block) &&
935+
!blocks[input.block].shadow;
936+
937+
if (!shadowIsBroken && !shadowIsMissing) continue;
938+
939+
// Try to find a peer block with a working shadow for this input.
940+
const template = findPeerShadow(blocks, block.opcode, inputName);
941+
if (!template && !shadowIsBroken) {
942+
// No peer has a shadow either — this input probably doesn't
943+
// use one (e.g. custom procedure inputs). Leave it alone.
944+
continue;
945+
}
946+
const shadowOpcode = template ? template.opcode : 'text';
947+
const primitiveInfo = primitiveOpcodeInfoMap[shadowOpcode];
948+
if (!primitiveInfo) {
949+
// Not a primitive shadow type — clear any stale
950+
// reference so it doesn't cause crashes.
951+
if (shadowIsBroken) input.shadow = null;
952+
continue;
953+
}
954+
const fieldName = primitiveInfo[1];
955+
const newShadowId = uid();
956+
blocks[newShadowId] = {
957+
id: newShadowId,
958+
opcode: shadowOpcode,
959+
next: null,
960+
parent: blockId,
961+
shadow: true,
962+
topLevel: false,
963+
inputs: Object.create(null),
964+
fields: {
965+
[fieldName]: {
966+
name: fieldName,
967+
value: ''
968+
}
969+
}
970+
};
971+
input.shadow = newShadowId;
972+
// If no block is connected, also set block to the shadow
973+
if (!input.block || !Object.prototype.hasOwnProperty.call(blocks, input.block)) {
974+
input.block = newShadowId;
975+
}
976+
}
977+
}
978+
888979
return blocks;
889980
};
890981

packages/scratch-vm/test/unit/engine_blocks.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,3 +1145,53 @@ test('getAllVariableAndListReferences returns broadcast when we tell it to', t =
11451145

11461146
t.end();
11471147
});
1148+
1149+
// Regression test for bug 878291: moveBlock should not crash when a
1150+
// shadow reference points to a block that does not exist.
1151+
test('moveBlock tolerates missing shadow block', t => {
1152+
const b = new Blocks(new Runtime());
1153+
1154+
// Create a parent block with an input whose shadow reference is stale
1155+
b.createBlock({
1156+
id: 'parent',
1157+
opcode: 'data_setvariableto',
1158+
next: null,
1159+
parent: null,
1160+
shadow: false,
1161+
topLevel: true,
1162+
inputs: {
1163+
VALUE: {
1164+
name: 'VALUE',
1165+
block: 'reporter',
1166+
shadow: 'nonexistent_shadow' // references a block that does not exist
1167+
}
1168+
},
1169+
fields: {}
1170+
});
1171+
b.createBlock({
1172+
id: 'reporter',
1173+
opcode: 'sensing_answer',
1174+
next: null,
1175+
parent: 'parent',
1176+
shadow: false,
1177+
topLevel: false,
1178+
inputs: {},
1179+
fields: {}
1180+
});
1181+
1182+
// Detaching the reporter should not throw even though the shadow is missing
1183+
t.doesNotThrow(() => {
1184+
b.moveBlock({
1185+
id: 'reporter',
1186+
oldParent: 'parent',
1187+
oldInput: 'VALUE',
1188+
newCoordinate: {x: 0, y: 0}
1189+
});
1190+
});
1191+
1192+
// The stale shadow reference should be cleared
1193+
t.equal(b.getBlock('parent').inputs.VALUE.shadow, null);
1194+
t.equal(b.getBlock('parent').inputs.VALUE.block, null);
1195+
1196+
t.end();
1197+
});

0 commit comments

Comments
 (0)