[api-minor] Ensure that the Array.prototype doesn't contain any enumerable properties
#11582
Conversation
…merable properties Over the years there's been a fair number of issues/PRs opened, where people have wanted to add `hasOwnProperty` checks in (hot) loops in the font parsing code. This has always been rejected, since we don't want to risk reducing performance in the Firefox PDF viewer simply because some users of the general PDF.js library are *incorrectly* extending the `Array.prototype` with enumerable properties. With this patch the general PDF.js library will now fail immediately with a hopefully useful Error message, rather than having (some) fonts fail to render, when the `Array.prototype` is incorrectly extended. Note that I did consider making this a warning, but ultimately decided against it since it's first of all possible to disable those (with the `verbosity` parameter). Secondly, even when printed, warnings can be easy to overlook and finally a warning may also *seem* OK to ignore (as opposed to an actual Error).
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/790ce34fe31f431/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/790ce34fe31f431/output.txt Total script time: 1.76 mins Published |
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/35b0cdc44f591fc/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/a94f488a489c40d/output.txt |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/a94f488a489c40d/output.txt Total script time: 2.09 mins
Image differences available at: http://54.215.176.217:8877/a94f488a489c40d/reftest-analyzer.html#web=eq.log |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/35b0cdc44f591fc/output.txt Total script time: 19.76 mins
Image differences available at: http://54.67.70.0:8877/35b0cdc44f591fc/reftest-analyzer.html#web=eq.log |
|
Looks like a good idea to me. Hopefully it will prevent some issues being created because of incorrectly extended array prototypes. Thanks! |
Over the years there's been a fair number of issues/PRs opened, where people have wanted to add
hasOwnPropertychecks in (hot) loops in the font parsing code. This has always been rejected, since we don't want to risk reducing performance in the Firefox PDF viewer simply because some users of the general PDF.js library are incorrectly extending theArray.prototypewith enumerable properties.With this patch the general PDF.js library will now fail immediately with a hopefully useful Error message, rather than having (some) fonts fail to render, when the
Array.prototypeis incorrectly extended.Note that I did consider making this a warning, but ultimately decided against it since it's first of all possible to disable those (with the
verbosityparameter). Secondly, even when printed, warnings can be easy to overlook and finally a warning may also seem OK to ignore (as opposed to an actual Error).