Skip to content

Commit fbca120

Browse files
committed
fix(fes): validate shader callbacks, prevent stack leaks, and remove dead code
1 parent 21a3d0e commit fbca120

6 files changed

Lines changed: 100 additions & 34 deletions

File tree

‎src/core/p5.Renderer3D.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2722,7 +2722,13 @@ function renderer3D(p5, fn) {
27222722
`buildComputeShader() is only available with the WebGPU renderer. ${webGPUAddonMessage}`
27232723
);
27242724
}
2725-
return this.baseComputeShader().modify(cb, context, { hook: 'iteration' });
2725+
const wasInternal = this._isUserCall;
2726+
this._isUserCall = false;
2727+
try {
2728+
return this.baseComputeShader().modify(cb, context, { hook: 'iteration' });
2729+
} finally {
2730+
this._isUserCall = wasInternal;
2731+
}
27262732
};
27272733
p5.registerDecorator('p5.prototype.buildComputeShader', markExperimental('webgpu', p5));
27282734

‎src/friendly_errors/param_validator.js‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ function validateParams(p5, fn, lifecycles) {
583583
errorStackParser.parse(Error()).slice(3)
584584
);
585585
const locationStr = getFriendlyStack(stacktrace, true);
586-
FES.log`${locationStr} ${message}`();
586+
FES.log`${locationStr ? locationStr + ' ' : ''}${message}`();
587587
}
588588
return message;
589589
};
@@ -630,18 +630,9 @@ function validateParams(p5, fn, lifecycles) {
630630
let funcSchemas = activeRegistry.get(func);
631631

632632
if (!funcSchemas) {
633-
// Convention-based bypass for shader builder entry-point functions.
634-
// All p5 shader builders follow the build*Shader naming convention
635-
// (buildMaterialShader, buildComputeShader, etc.). They are called
636-
// BEFORE the strands context activates and handle their own validation
637-
// and error reporting internally (e.g. "only available with WebGPU").
638-
if (/^build\w+Shader$/.test(func) && p5._isStrandsContextActive) {
639-
funcSchemas = z.any();
640-
}
641633
// Check _strandsSignatures for explicit bypass entries (value === true)
642-
// regardless of strands context, or array-based overload schemas that
643-
// only apply inside an active strands context.
644-
else if (p5._isStrandsContextActive && p5._strandsSignatures?.has(func)) {
634+
// that only apply inside an active strands context.
635+
if (p5._isStrandsContextActive && p5._strandsSignatures?.has(func)) {
645636
funcSchemas = z.any();
646637
}
647638

‎src/friendly_errors/stacktrace.js‎

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -279,27 +279,29 @@ export const processStack = (error, stacktrace) => {
279279
let isInternal = false;
280280
let p5FileName, friendlyStack;
281281

282+
// Helper to strip query strings and hashes from URLs for consistent matching
283+
const cleanUrl = url => (typeof url === 'string' ? url.split('?')[0].split('#')[0] : '');
284+
282285
// Intentionally throw an error that we catch so that we can check the name
283286
// of the current file. Any errors we see from this file, we treat as
284287
// internal errors.
285288
const testStacktrace = errorStackParser.parse(Error());
286-
p5FileName = testStacktrace[0].fileName;
289+
p5FileName = cleanUrl(testStacktrace[0]?.fileName);
290+
const p5BaseName = p5FileName ? p5FileName.split('/').slice(-1)[0] : '';
291+
const isP5File = url => {
292+
const cleaned = cleanUrl(url);
293+
if (!cleaned) return false;
294+
if (cleaned === p5FileName) return true;
295+
if (p5BaseName && cleaned.split('/').slice(-1)[0] === p5BaseName) return true;
296+
return false;
297+
};
287298

288299
for (let i = stacktrace.length - 1; i >= 0; i--) {
289300
let splitted = stacktrace[i].functionName.split('.');
290301
if (entryPoints.includes(splitted[splitted.length - 1])) {
291302
// remove everything below an entry point function (setup, draw, etc).
292303
// (it's usually the internal initialization calls)
293304
friendlyStack = stacktrace.slice(0, i + 1);
294-
// We call the error "internal" if the source of the error was a
295-
// function from within the p5.js library file, but called from the
296-
// user's code directly. We only need to check the topmost frame in
297-
// the stack trace since any function internal to p5 should pass this
298-
// check, not just public p5 functions.
299-
if (stacktrace[0].fileName === p5FileName) {
300-
isInternal = true;
301-
break;
302-
}
303305
break;
304306
}
305307
}
@@ -309,6 +311,14 @@ export const processStack = (error, stacktrace) => {
309311
// entire stacktrace for friendlyStack
310312
if (!friendlyStack) friendlyStack = stacktrace;
311313

314+
// We call the error "internal" if the source of the error was a
315+
// function from within the p5.js library file, but called from the
316+
// user's code directly. When error is null (called from param_validator),
317+
// it is always an internal validation call.
318+
if (error === null || isP5File(stacktrace[0]?.fileName)) {
319+
isInternal = true;
320+
}
321+
312322
if (isInternal) {
313323
// the frameIndex property is added before the filter, so frameIndex
314324
// corresponds to the index of a frame in the original stacktrace.
@@ -319,7 +329,15 @@ export const processStack = (error, stacktrace) => {
319329
frame.frameIndex = index;
320330
return frame;
321331
})
322-
.filter(frame => frame.fileName !== p5FileName);
332+
.filter(frame => {
333+
const cleaned = cleanUrl(frame.fileName);
334+
return (
335+
cleaned &&
336+
!isP5File(frame.fileName) &&
337+
cleaned !== '<anonymous>' &&
338+
!cleaned.startsWith('eval')
339+
);
340+
});
323341

324342
// a weird case, if for some reason we can't identify the function called
325343
// from user's code
@@ -332,16 +350,19 @@ export const processStack = (error, stacktrace) => {
332350
friendlyStack[0].lineNumber &&
333351
friendlyStack[0].columnNumber
334352
) {
353+
const cleanFile = cleanUrl(friendlyStack[0].fileName);
335354
locationObj = {
336-
location: `${friendlyStack[0].fileName}:${
355+
location: `${cleanFile}:${
337356
friendlyStack[0].lineNumber
338357
}:${friendlyStack[0].columnNumber}`,
339-
file: friendlyStack[0].fileName.split('/').slice(-1),
358+
file: cleanFile.split('/').slice(-1)[0],
340359
line: friendlyStack[0].lineNumber
341360
};
342361

343362
// if already handled by another part of the FES, don't handle again
344-
if (p5._fesLogCache[locationObj.location]) return [true, null];
363+
if (typeof p5 !== 'undefined' && p5?._fesLogCache?.[locationObj.location]) {
364+
return [true, null];
365+
}
345366
}
346367

347368
// Library error
@@ -394,8 +415,16 @@ export function printFriendlyStack(friendlyStack) {
394415
}
395416

396417
export function getFriendlyStack(stacktrace, compact = false) {
418+
if (!stacktrace || !stacktrace.length || !stacktrace[0]?.fileName) {
419+
return null;
420+
}
421+
const rawFile = stacktrace[0].fileName;
422+
const cleanFile = (typeof rawFile === 'string' ? rawFile.split('?')[0].split('#')[0] : '').split('/').slice(-1)[0];
423+
if (!cleanFile || cleanFile === '<anonymous>' || cleanFile.startsWith('eval')) {
424+
return null;
425+
}
397426
if (compact) {
398-
return `[${stacktrace[0].fileName.split('/').slice(-1)}, line ${stacktrace[0].lineNumber}]`;
427+
return `[${cleanFile}, line ${stacktrace[0].lineNumber}]`;
399428
} else {
400429
if (stacktrace.length > 1) {
401430
let stacktraceMsg = '';

‎src/strands/strands_api.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,7 @@ export function initGlobalStrandsAPI(p5, fn, strandsContext) {
482482
strandsBuiltinFunctions
483483
)) {
484484
p5._strandsSignatures.set(functionName, true);
485+
p5._strandsSignatures.set(`p5.${functionName}`, true);
485486
const isp5Function = overrides[0].isp5Function;
486487
if (isp5Function) {
487488
const originalFn = fn[functionName];

‎src/webgl/material.js‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -745,7 +745,13 @@ function material(p5, fn) {
745745
* @returns {p5.Shader} The material shader
746746
*/
747747
fn.buildFilterShader = function (callback, scope) {
748-
return this.baseFilterShader().modify(callback, scope);
748+
const wasInternal = this._isUserCall;
749+
this._isUserCall = false;
750+
try {
751+
return this.baseFilterShader().modify(callback, scope);
752+
} finally {
753+
this._isUserCall = wasInternal;
754+
}
749755
};
750756
p5.registerDecorator('p5.prototype.buildFilterShader', markExperimental('p5.strands', p5));
751757

@@ -1574,7 +1580,13 @@ function material(p5, fn) {
15741580
* @returns {p5.Shader} The material shader.
15751581
*/
15761582
fn.buildMaterialShader = function (cb, scope) {
1577-
return this.baseMaterialShader().modify(cb, scope);
1583+
const wasInternal = this._isUserCall;
1584+
this._isUserCall = false; // Temporarily un-suppress FES so calls inside cb are validated
1585+
try {
1586+
return this.baseMaterialShader().modify(cb, scope);
1587+
} finally {
1588+
this._isUserCall = wasInternal;
1589+
}
15781590
};
15791591
p5.registerDecorator('p5.prototype.buildMaterialShader', markExperimental('p5.strands', p5));
15801592

@@ -1793,7 +1805,13 @@ function material(p5, fn) {
17931805
* @returns {p5.Shader} The normal shader.
17941806
*/
17951807
fn.buildNormalShader = function (cb, scope) {
1796-
return this.baseNormalShader().modify(cb, scope);
1808+
const wasInternal = this._isUserCall;
1809+
this._isUserCall = false;
1810+
try {
1811+
return this.baseNormalShader().modify(cb, scope);
1812+
} finally {
1813+
this._isUserCall = wasInternal;
1814+
}
17971815
};
17981816
p5.registerDecorator('p5.prototype.buildNormalShader', markExperimental('p5.strands', p5));
17991817

@@ -1958,7 +1976,13 @@ function material(p5, fn) {
19581976
* @returns {p5.Shader} The color shader.
19591977
*/
19601978
fn.buildColorShader = function (cb, scope) {
1961-
return this.baseColorShader().modify(cb, scope);
1979+
const wasInternal = this._isUserCall;
1980+
this._isUserCall = false;
1981+
try {
1982+
return this.baseColorShader().modify(cb, scope);
1983+
} finally {
1984+
this._isUserCall = wasInternal;
1985+
}
19621986
};
19631987
p5.registerDecorator('p5.prototype.buildColorShader', markExperimental('p5.strands', p5));
19641988

@@ -2216,7 +2240,13 @@ function material(p5, fn) {
22162240
* @returns {p5.Shader} The stroke shader.
22172241
*/
22182242
fn.buildStrokeShader = function (cb, scope) {
2219-
return this.baseStrokeShader().modify(cb, scope);
2243+
const wasInternal = this._isUserCall;
2244+
this._isUserCall = false;
2245+
try {
2246+
return this.baseStrokeShader().modify(cb, scope);
2247+
} finally {
2248+
this._isUserCall = wasInternal;
2249+
}
22202250
};
22212251
p5.registerDecorator('p5.prototype.buildStrokeShader', markExperimental('p5.strands', p5));
22222252

‎test/unit/core/param_errors.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ suite('Validate Params', function () {
2929
return 'mock p5.Graphics';
3030
},
3131
_error: () => {},
32-
registerDecorator: () => {}
32+
registerDecorator: () => {},
33+
_strandsSignatures: new Map([['p5.map', true]]),
34+
_isStrandsContextActive: false
3335
};
3436
const mockP5Prototype = {};
3537

@@ -469,5 +471,12 @@ suite('Validate Params', function () {
469471
assert.isFalse(result.success);
470472
assert.isTrue(result.error.toString().includes('Expected at least 5 arguments'));
471473
});
474+
475+
test('map(): bypasses FES validation when inside strands context', function () {
476+
mockP5._isStrandsContextActive = true;
477+
const result = mockP5Prototype._validate('p5.map', ['a', 'b']);
478+
assert.isTrue(result.success);
479+
mockP5._isStrandsContextActive = false; // reset
480+
});
472481
});
473482
});

0 commit comments

Comments
 (0)