From 8cb53af30a8dcbf373138b2b2d183262d195c457 Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 24 Oct 2024 18:55:21 +0100 Subject: [PATCH 1/7] search: check that query terms exist as properties in term indices before accessing them --- sphinx/themes/basic/static/searchtools.js | 4 ++-- tests/js/fixtures/ecmascript/searchindex.js | 1 + tests/js/roots/ecmascript/conf.py | 0 tests/js/roots/ecmascript/index.rst | 5 +++++ 4 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 tests/js/fixtures/ecmascript/searchindex.js create mode 100644 tests/js/roots/ecmascript/conf.py create mode 100644 tests/js/roots/ecmascript/index.rst diff --git a/sphinx/themes/basic/static/searchtools.js b/sphinx/themes/basic/static/searchtools.js index aaf078d2b91..f13f5027934 100644 --- a/sphinx/themes/basic/static/searchtools.js +++ b/sphinx/themes/basic/static/searchtools.js @@ -514,8 +514,8 @@ const Search = { searchTerms.forEach((word) => { const files = []; const arr = [ - { files: terms[word], score: Scorer.term }, - { files: titleTerms[word], score: Scorer.title }, + { files: terms.hasOwnProperty(word) ? terms[word] : [], score: Scorer.term }, + { files: titleTerms.hasOwnProperty(word) ? titleTerms[word] : [], score: Scorer.title }, ]; // add support for partial matches if (word.length > 2) { diff --git a/tests/js/fixtures/ecmascript/searchindex.js b/tests/js/fixtures/ecmascript/searchindex.js new file mode 100644 index 00000000000..f878089d88d --- /dev/null +++ b/tests/js/fixtures/ecmascript/searchindex.js @@ -0,0 +1 @@ +Search.setIndex({"alltitles":{},"docnames":["index"],"envversion":{"sphinx":64,"sphinx.domains.c":3,"sphinx.domains.changeset":1,"sphinx.domains.citation":1,"sphinx.domains.cpp":9,"sphinx.domains.index":1,"sphinx.domains.javascript":3,"sphinx.domains.math":2,"sphinx.domains.python":4,"sphinx.domains.rst":2,"sphinx.domains.std":2},"filenames":["index.rst"],"indexentries":{"object.__proto__ (object attribute)":[[0,"Object.__proto__",false]]},"objects":{"Object":[[0,0,1,"","__proto__"]]},"objnames":{"0":["js","attribute","JavaScript attribute"]},"objtypes":{"0":"js:attribute"},"terms":{"__proto__":0,"access":0,"aka":0,"an":0,"ecmascript":0,"engin":0,"fixtur":0,"gener":0,"i":0,"index":0,"instanc":0,"javascript":0,"object":0,"project":0,"prototyp":0,"sampl":0,"search":0,"thi":0,"us":0},"titles":["<no title>"],"titleterms":{}}) \ No newline at end of file diff --git a/tests/js/roots/ecmascript/conf.py b/tests/js/roots/ecmascript/conf.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/js/roots/ecmascript/index.rst b/tests/js/roots/ecmascript/index.rst new file mode 100644 index 00000000000..6f9e9886912 --- /dev/null +++ b/tests/js/roots/ecmascript/index.rst @@ -0,0 +1,5 @@ +This is a sample JavaScript (aka ``ECMAScript``) project used to generate a search engine index fixture. + +.. js:attribute:: Object.__proto__ + + Used to access the `prototype `_ of an object instance. From d10d786470a51983da52ebbf95fba9ea75024502 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 5 Nov 2024 22:07:06 +0000 Subject: [PATCH 2/7] Tests: add negative-test case The JS prototype property should _not_ be found in unrelated documents --- tests/js/searchtools.spec.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/js/searchtools.spec.js b/tests/js/searchtools.spec.js index cfe5fdcf7ed..2ee55ad00ac 100644 --- a/tests/js/searchtools.spec.js +++ b/tests/js/searchtools.spec.js @@ -209,6 +209,19 @@ describe('Basic html theme search', function() { }); + describe('can handle edge-case search queries', function() { + + it('does not find the javascript prototype property in unrelated documents', function() { + eval(loadFixture("partial/searchindex.js")); + + searchParameters = Search._parseQuery('__proto__'); + + hits = []; + expect(Search._performSearch(...searchParameters)).toEqual(hits); + }); + + }); + }); describe("htmlToText", function() { From 24ef507c9c695d7375cc08df9ad0040e75f276cf Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 11 Nov 2024 17:27:59 +0000 Subject: [PATCH 3/7] search: add explanatory comment for document title/text term lookup --- sphinx/themes/basic/static/searchtools.js | 1 + 1 file changed, 1 insertion(+) diff --git a/sphinx/themes/basic/static/searchtools.js b/sphinx/themes/basic/static/searchtools.js index f13f5027934..076dd33ec04 100644 --- a/sphinx/themes/basic/static/searchtools.js +++ b/sphinx/themes/basic/static/searchtools.js @@ -513,6 +513,7 @@ const Search = { // perform the search on the required terms searchTerms.forEach((word) => { const files = []; + // find documents, if any, containing the query word in their text/title term indices const arr = [ { files: terms.hasOwnProperty(word) ? terms[word] : [], score: Scorer.term }, { files: titleTerms.hasOwnProperty(word) ? titleTerms[word] : [], score: Scorer.title }, From a18541c043e13f6872a4cc9656ed6b8c045a7bcd Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 11 Nov 2024 17:29:27 +0000 Subject: [PATCH 4/7] search: retain `undefined` result instead of `[]` for non-matching terms --- sphinx/themes/basic/static/searchtools.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sphinx/themes/basic/static/searchtools.js b/sphinx/themes/basic/static/searchtools.js index 076dd33ec04..41cb5f996f2 100644 --- a/sphinx/themes/basic/static/searchtools.js +++ b/sphinx/themes/basic/static/searchtools.js @@ -515,8 +515,8 @@ const Search = { const files = []; // find documents, if any, containing the query word in their text/title term indices const arr = [ - { files: terms.hasOwnProperty(word) ? terms[word] : [], score: Scorer.term }, - { files: titleTerms.hasOwnProperty(word) ? titleTerms[word] : [], score: Scorer.title }, + { files: terms.hasOwnProperty(word) ? terms[word] : undefined, score: Scorer.term }, + { files: titleTerms.hasOwnProperty(word) ? titleTerms[word] : undefined, score: Scorer.title }, ]; // add support for partial matches if (word.length > 2) { From 3d605e032975f6c67f9ed34f3dfd3aa1935ac20e Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 11 Nov 2024 17:30:42 +0000 Subject: [PATCH 5/7] search: add explanatory comment regarding `hasOwnProperty` usage --- sphinx/themes/basic/static/searchtools.js | 1 + 1 file changed, 1 insertion(+) diff --git a/sphinx/themes/basic/static/searchtools.js b/sphinx/themes/basic/static/searchtools.js index 41cb5f996f2..91f4be57fc8 100644 --- a/sphinx/themes/basic/static/searchtools.js +++ b/sphinx/themes/basic/static/searchtools.js @@ -514,6 +514,7 @@ const Search = { searchTerms.forEach((word) => { const files = []; // find documents, if any, containing the query word in their text/title term indices + // use Object.hasOwnProperty to avoid mismatching against prototype properties const arr = [ { files: terms.hasOwnProperty(word) ? terms[word] : undefined, score: Scorer.term }, { files: titleTerms.hasOwnProperty(word) ? titleTerms[word] : undefined, score: Scorer.title }, From fa2ec64dab60ba5a1c0fae9ff8293df3f9ea2669 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 24 Nov 2024 16:41:30 +0000 Subject: [PATCH 6/7] Add CHANGES.rst entry --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 86e59279491..8021da4873d 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -26,6 +26,8 @@ Bugs fixed * LaTeX: fix a ``7.4.0`` typo in a default for ``\sphinxboxsetup`` (refs: PR #13152). Patch by Jean-François B. +* #13096: HTML Search: check that query terms exist as properties in + term indices before accessing them. Testing ------- From 5881a9896c5b703dc777aea8acfab9f941452869 Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 24 Nov 2024 16:44:31 +0000 Subject: [PATCH 7/7] Tests: cleanup: remove `ecmascript` test root and fixtures --- tests/js/fixtures/ecmascript/searchindex.js | 1 - tests/js/roots/ecmascript/conf.py | 0 tests/js/roots/ecmascript/index.rst | 5 ----- 3 files changed, 6 deletions(-) delete mode 100644 tests/js/fixtures/ecmascript/searchindex.js delete mode 100644 tests/js/roots/ecmascript/conf.py delete mode 100644 tests/js/roots/ecmascript/index.rst diff --git a/tests/js/fixtures/ecmascript/searchindex.js b/tests/js/fixtures/ecmascript/searchindex.js deleted file mode 100644 index f878089d88d..00000000000 --- a/tests/js/fixtures/ecmascript/searchindex.js +++ /dev/null @@ -1 +0,0 @@ -Search.setIndex({"alltitles":{},"docnames":["index"],"envversion":{"sphinx":64,"sphinx.domains.c":3,"sphinx.domains.changeset":1,"sphinx.domains.citation":1,"sphinx.domains.cpp":9,"sphinx.domains.index":1,"sphinx.domains.javascript":3,"sphinx.domains.math":2,"sphinx.domains.python":4,"sphinx.domains.rst":2,"sphinx.domains.std":2},"filenames":["index.rst"],"indexentries":{"object.__proto__ (object attribute)":[[0,"Object.__proto__",false]]},"objects":{"Object":[[0,0,1,"","__proto__"]]},"objnames":{"0":["js","attribute","JavaScript attribute"]},"objtypes":{"0":"js:attribute"},"terms":{"__proto__":0,"access":0,"aka":0,"an":0,"ecmascript":0,"engin":0,"fixtur":0,"gener":0,"i":0,"index":0,"instanc":0,"javascript":0,"object":0,"project":0,"prototyp":0,"sampl":0,"search":0,"thi":0,"us":0},"titles":["<no title>"],"titleterms":{}}) \ No newline at end of file diff --git a/tests/js/roots/ecmascript/conf.py b/tests/js/roots/ecmascript/conf.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/tests/js/roots/ecmascript/index.rst b/tests/js/roots/ecmascript/index.rst deleted file mode 100644 index 6f9e9886912..00000000000 --- a/tests/js/roots/ecmascript/index.rst +++ /dev/null @@ -1,5 +0,0 @@ -This is a sample JavaScript (aka ``ECMAScript``) project used to generate a search engine index fixture. - -.. js:attribute:: Object.__proto__ - - Used to access the `prototype `_ of an object instance.