#7391 circular blocks

cypress/e2e/circular-blocks.cy.js

@@ -0,0 +1,62 @@
+describe("circular connection validation in loadNewBlocks", () => {
+    it("rejects multi-block circular connection projects", () => {
+        cy.visit("http://127.0.0.1:3000/", {
+            onBeforeLoad(win) {
+                cy.stub(win.console, "debug").as("consoleDebug");
+            }
+        });
+
+        // Wait for page to load
+        cy.get("#myCanvas", { timeout: 30000 }).should("be.visible");
+
+        cy.window().then(win => {
+            const activity = win.globalActivity || win.ActivityContext.getActivity();
+            expect(activity).to.exist;
+
+            const maliciousProject = [
+                [0, "start", 100, 100, [null, 1, null]],
+                [1, "action", 200, 100, [null, 0, null]]
+            ];
+
+            activity.blocks.loadNewBlocks(maliciousProject);
+
+            cy.get("@consoleDebug").should(
+                "have.been.calledWithMatch",
+                /Circular connection in block data/
+            );
+            cy.get("@consoleDebug").should(
+                "have.been.calledWith",
+                "Punting loading of new blocks!"
+            );
+        });
+    });
+
+    it("allows valid acyclic project blocks", () => {
+        cy.visit("http://127.0.0.1:3000/", {
+            onBeforeLoad(win) {
+                cy.stub(win.console, "debug").as("consoleDebug");
+            }
+        });
+
+        // Wait for page to load
+        cy.get("#myCanvas", { timeout: 30000 }).should("be.visible");
+
+        cy.window().then(win => {
+            const activity = win.globalActivity || win.ActivityContext.getActivity();
+            expect(activity).to.exist;
+
+            const validProject = [
+                [0, "start", 100, 100, [null, 1, null]],
+                [1, "forward", 200, 100, [0, null, null]]
+            ];
+
+            activity.blocks.loadNewBlocks(validProject);
+
+            // It should not log circular connection messages
+            cy.get("@consoleDebug").should(
+                "not.have.been.calledWithMatch",
+                /Circular connection in block data/
+            );
+        });
+    });
+});

js/__tests__/blocks.test.js

@@ -621,4 +621,88 @@ describe("Blocks Foundation", () => {
             expect(paletteBlock.defaults[0]).toBe("jump");
         });
     });
+
+    describe("loadNewBlocks", () => {
+        let blocksInstance;
+        let mockActivity;
+        let debugSpy;
+
+        beforeEach(() => {
+            mockActivity = {
+                turtles: {
+                    getTurtle: jest.fn(),
+                    getTurtleCount: jest.fn().mockReturnValue(0)
+                },
+                palettes: {
+                    dict: {
+                        action: { protoList: [] }
+                    }
+                }
+            };
+            blocksInstance = new Blocks(mockActivity);
+            blocksInstance.blockList = [];
+            debugSpy = jest.spyOn(console, "debug").mockImplementation(() => {});
+        });
+
+        afterEach(() => {
+            debugSpy.mockRestore();
+        });
+
+        it("should load valid acyclic project blocks without pouting/aborting", () => {
+            const project = [
+                [0, "start", 100, 100, [null, 1, null]],
+                [1, "forward", 200, 100, [0, null, null]]
+            ];
+            const processSpy = jest
+                .spyOn(blocksInstance, "_processOneBlock")
+                .mockImplementation(() => {});
+
+            blocksInstance.loadNewBlocks(project);
+
+            expect(debugSpy).not.toHaveBeenCalledWith(
+                expect.stringContaining("Circular connection in block data")
+            );
+            processSpy.mockRestore();
+        });
+
+        it("should reject direct self-loop (A -> A)", () => {
+            const project = [[0, "start", 100, 100, [0, null, null]]];
+
+            blocksInstance.loadNewBlocks(project);
+
+            expect(debugSpy).toHaveBeenCalledWith(
+                expect.stringContaining("Circular connection in block data")
+            );
+            expect(debugSpy).toHaveBeenCalledWith("Punting loading of new blocks!");
+        });
+
+        it("should reject dual-block cycle (A -> B -> A)", () => {
+            const project = [
+                [0, "start", 100, 100, [null, 1, null]],
+                [1, "action", 200, 100, [null, 0, null]]
+            ];
+
+            blocksInstance.loadNewBlocks(project);
+
+            expect(debugSpy).toHaveBeenCalledWith(
+                expect.stringContaining("Circular connection in block data")
+            );
+            expect(debugSpy).toHaveBeenCalledWith("Punting loading of new blocks!");
+        });
+
+        it("should reject multi-block cycle (A -> B -> C -> A)", () => {
+            const project = [
+                [0, "start", 100, 100, [null, 1, null]],
+                [1, "action", 200, 100, [null, 2, null]],
+                [2, "forward", 300, 100, [null, 0, null]]
+            ];
+
+            blocksInstance.loadNewBlocks(project);
+
+            expect(debugSpy).toHaveBeenCalledWith(
+                expect.stringContaining("Circular connection in block data")
+            );
+            expect(debugSpy).toHaveBeenCalledWith("Punting loading of new blocks!");
+        });
+    });
 });

js/blocks.js

@@ -5794,23 +5794,78 @@ class Blocks {
                 blockObjs.pop();
             }
 
-            /** Check for blocks connected to themselves, */
-            /** and for action blocks not connected to text blocks. */
+            /** Check for circular connections in block data. */
+            // 1. Direct self-loops
             for (let b = 0; b < blockObjs.length; b++) {
                 const blkData = blockObjs[b];
-
                 for (const c in blkData[4]) {
                     if (blkData[4][c] === blkData[0]) {
                         console.debug("Circular connection in block data: " + blkData);
-
                         console.debug("Punting loading of new blocks!");
-
                         console.debug(blockObjs);
                         return;
                     }
                 }
             }
 
+            // 2. Multi-block cycles (DFS on outgoing connections: indices 1 and above)
+            const hasCycle = () => {
+                const adj = new Map();
+                for (let b = 0; b < blockObjs.length; b++) {
+                    const blkData = blockObjs[b];
+                    const id = blkData[0];
+                    const connections = blkData[4] || [];
+                    const neighbors = [];
+                    for (let c = 1; c < connections.length; c++) {
+                        const connId = connections[c];
+                        if (connId !== null && connId !== undefined) {
+                            neighbors.push(connId);
+                        }
+                    }
+                    adj.set(id, neighbors);
+                }
+
+                const visited = new Set();
+                const recStack = new Set();
+
+                const dfs = nodeId => {
+                    visited.add(nodeId);
+                    recStack.add(nodeId);
+
+                    const neighbors = adj.get(nodeId) || [];
+                    for (let i = 0; i < neighbors.length; i++) {
+                        const neighborId = neighbors[i];
+                        if (!visited.has(neighborId)) {
+                            if (dfs(neighborId)) {
+                                return true;
+                            }
+                        } else if (recStack.has(neighborId)) {
+                            return true;
+                        }
+                    }
+
+                    recStack.delete(nodeId);
+                    return false;
+                };
+
+                for (let b = 0; b < blockObjs.length; b++) {
+                    const id = blockObjs[b][0];
+                    if (!visited.has(id)) {
+                        if (dfs(id)) {
+                            return true;
+                        }
+                    }
+                }
+                return false;
+            };
+
+            if (hasCycle()) {
+                console.debug("Circular connection in block data");
+                console.debug("Punting loading of new blocks!");
+                console.debug(blockObjs);
+                return;
+            }
+
             /** We'll need a list of existing storein and action names. */
             const currentActionNames = [];
             const currentStoreinNames = [];