-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
HTML search: sanitise 'searchindex.js' contents #13099
HTML search: sanitise 'searchindex.js' contents #13099
Conversation
* Remove the JavaScript `Object` prototype from most objects (with the exception of `Array` objects, where the prototype functions are useful). * Freeze the resulting search index entries (this does also prevent modifications to the previously mentioned `Array` objects). Note: some JavaScript built-in types such as `Set` and `Map` permit modifications to their contents even after they are frozen. However, they should not appear in the index contents since it is represented as JSON by the Python code in the `sphinx.search` module.
[skip ci]
As a side-effect, this introduces a limitation: the argument passed to the initial `sanitiseRecursive` call must be an `Object` itself.
In particular, relocates the `Array.isArray` conditional clause to reduce the recursion depth by one level.
This comment was marked as outdated.
This comment was marked as outdated.
I have to be honest: I'm not sure that I can achieve this realistically within reasonable I'd be reluctant to close/leave an attempt to implement some (what I consider) safety improvements unfinished -- so in fact I probably will work more on this during the week -- but I have doubts about whether it can be completed in a satisfying way. |
Relates-to commit 0e61a3c. [skip ci]
The most recent two performance traces I've run using Firefox and Chromium respectively put the traced duration for I don't have any further changes planned on this branch currently. |
Despite the fact that the performance overhead introduced by these changes may not affect the perceptible time-to-appearance of search result display for Sphinx HTML project builds, I'm going to close this because that overhead, on aggregate, I think would add a nontrivial resource cost. If the change eliminated a category of security problems, then perhaps that would be worthwhile -- but in this case, we don't have a known problem to resolve. I will hold the related issue #13098 open, because I think in future there may be methods (most likely Records and Tuples) to achieve search index immutability with minimal runtime performance cost (or perhaps even benefits, if resulting compiler/bytecode optimizations become possible thanks to the immutability). |
(if for some reason the code from this branch does prove useful in future, feel free to continue on from it. in particular I would also recommend considering removing the prototype from |
Feature or Bugfix
Purpose
Detail
Object
prototype from most objects (with the exception ofArray
objects, where the prototype functions are useful).Array
objects).Relates
hasOwnProperty
from the objects in the search index).