From 4f67b483c0f436bdbdc57ed822c2894ec74110ec Mon Sep 17 00:00:00 2001 From: Maxime Janton <127086@supinfo.com> Date: Wed, 31 May 2017 00:49:09 +0200 Subject: [PATCH] feat(hits): opt-in xss filtering for hits and infinite hits. FIX #2138 * feat(connectHits): handle XSS and escape HTML entities * test(connectHits): ensure hit are correctly escaped * feat(connectHits): remove flagged boolean * feat(connectHits): add `escapeHits` and `escapeHitsWhitelist` options * feat(hitsWidget): support `escapeHits` and `escapeHitsWhitelist * test(hitsWidget): should throw on incorrect options * fix(connectHits): replace from objects into highlightProperties * fix(connectHits): replace Array.includes with Array.indexOf * refactor(escapeHits): export function, escape only highlight * refactor(connectHits): use `escapeHighlight` helper * refactor(connectHits): use tag config from `escapeHighlight * test(connectHits): separated test for escape highlight property * refactor(connectHits): always escape highlight property * feat(connectInfiniteHits): always escape highlight property * fix(escape-highlight): ensure `results.hits` is escaped only once * feat(hits): opt-in for escaping hits * feat(infinite-hits): opt-in for escape * test(hits): remove un-used test --- .../hits/__tests__/connectHits-test.js | 62 +++++++- src/connectors/hits/connectHits.js | 22 ++- .../__tests__/connectInfiniteHits-test.js | 57 +++++++- .../infinite-hits/connectInfiniteHits.js | 20 ++- src/lib/__tests__/escape-highlight-test.js | 136 ++++++++++++++++++ src/lib/escape-highlight.js | 54 +++++++ src/widgets/hits/hits.js | 7 +- .../__tests__/infinite-hits-test.js | 11 +- src/widgets/infinite-hits/infinite-hits.js | 8 +- 9 files changed, 361 insertions(+), 16 deletions(-) create mode 100644 src/lib/__tests__/escape-highlight-test.js create mode 100644 src/lib/escape-highlight.js diff --git a/src/connectors/hits/__tests__/connectHits-test.js b/src/connectors/hits/__tests__/connectHits-test.js index 16160c15bb..755692ffee 100644 --- a/src/connectors/hits/__tests__/connectHits-test.js +++ b/src/connectors/hits/__tests__/connectHits-test.js @@ -15,7 +15,10 @@ describe('connectHits', () => { const makeWidget = connectHits(rendering); const widget = makeWidget(); - expect(widget.getConfiguration).toEqual(undefined); + expect(widget.getConfiguration()).toEqual({ + highlightPreTag: '__ais-highlight__', + highlightPostTag: '__/ais-highlight__', + }); // test if widget is not rendered yet at this point expect(rendering.callCount).toBe(0); @@ -52,7 +55,7 @@ describe('connectHits', () => { it('Provides the hits and the whole results', () => { const rendering = sinon.stub(); const makeWidget = connectHits(rendering); - const widget = makeWidget(); + const widget = makeWidget({}); const helper = jsHelper(fakeClient, '', {}); helper.search = sinon.stub(); @@ -72,7 +75,8 @@ describe('connectHits', () => { {fake: 'data'}, {sample: 'infos'}, ]; - const results = new SearchResults(helper.state, [{hits}]); + + const results = new SearchResults(helper.state, [{hits: [].concat(hits)}]); widget.render({ results, state: helper.state, @@ -84,4 +88,56 @@ describe('connectHits', () => { expect(secondRenderingOptions.hits).toEqual(hits); expect(secondRenderingOptions.results).toEqual(results); }); + + it('escape highlight properties if requested', () => { + const rendering = sinon.stub(); + const makeWidget = connectHits(rendering); + const widget = makeWidget({escapeHits: true}); + + const helper = jsHelper(fakeClient, '', {}); + helper.search = sinon.stub(); + + widget.init({ + helper, + state: helper.state, + createURL: () => '#', + onHistoryChange: () => {}, + }); + + const firstRenderingOptions = rendering.lastCall.args[0]; + expect(firstRenderingOptions.hits).toEqual([]); + expect(firstRenderingOptions.results).toBe(undefined); + + const hits = [ + { + _highlightResult: { + foobar: { + value: '', + }, + }, + }, + ]; + + const results = new SearchResults(helper.state, [{hits}]); + widget.render({ + results, + state: helper.state, + helper, + createURL: () => '#', + }); + + const escapedHits = [ + { + _highlightResult: { + foobar: { + value: '<script>foobar</script>', + }, + }, + }, + ]; + + const secondRenderingOptions = rendering.lastCall.args[0]; + expect(secondRenderingOptions.hits).toEqual(escapedHits); + expect(secondRenderingOptions.results).toEqual(results); + }); }); diff --git a/src/connectors/hits/connectHits.js b/src/connectors/hits/connectHits.js index ac3489c92d..4c574665fb 100644 --- a/src/connectors/hits/connectHits.js +++ b/src/connectors/hits/connectHits.js @@ -1,3 +1,4 @@ +import escapeHits, {tagConfig} from '../../lib/escape-highlight.js'; import {checkRendering} from '../../lib/utils.js'; const usage = `Usage: @@ -9,7 +10,11 @@ var customHits = connectHits(function render(params, isFirstRendering) { // widgetParams, // } }); -search.addWidget(customHits()); +search.addWidget( + customHits({ + [ escapeHits = false ] + }) +); Full documentation available at https://community.algolia.com/instantsearch.js/connectors/connectHits.html `; @@ -20,11 +25,16 @@ Full documentation available at https://community.algolia.com/instantsearch.js/c * @property {Object} widgetParams All original widget options forwarded to the `renderFn`. */ +/** + * @typedef {Object} CustomHitsWidgetOptions + * @property {boolean} [escapeHits = false] If true, escape HTML tags from `hits[i]._highlightResult`. + */ + /** * **Hits** connector provides the logic to create custom widgets that will render the results retrieved from Algolia. * @type {Connector} * @param {function(HitsRenderingOptions, boolean)} renderFn Rendering function for the custom **Hits** widget. - * @return {function} Re-usable widget factory for a custom **Hits** widget. + * @return {function(CustomHitsWidgetOptions)} Re-usable widget factory for a custom **Hits** widget. * @example * // custom `renderFn` to render the custom Hits widget * function renderFn(HitsRenderingOptions) { @@ -49,6 +59,10 @@ export default function connectHits(renderFn) { checkRendering(renderFn, usage); return (widgetParams = {}) => ({ + getConfiguration() { + return tagConfig; + }, + init({instantSearchInstance}) { renderFn({ hits: [], @@ -59,6 +73,10 @@ export default function connectHits(renderFn) { }, render({results, instantSearchInstance}) { + if (widgetParams.escapeHits && results.hits && results.hits.length > 0) { + results.hits = escapeHits(results.hits); + } + renderFn({ hits: results.hits, results, diff --git a/src/connectors/infinite-hits/__tests__/connectInfiniteHits-test.js b/src/connectors/infinite-hits/__tests__/connectInfiniteHits-test.js index c4e4b8d3d5..ca73c35bbe 100644 --- a/src/connectors/infinite-hits/__tests__/connectInfiniteHits-test.js +++ b/src/connectors/infinite-hits/__tests__/connectInfiniteHits-test.js @@ -17,7 +17,10 @@ describe('connectInfiniteHits', () => { hitsPerPage: 10, }); - expect(widget.getConfiguration).toEqual(undefined); + expect(widget.getConfiguration()).toEqual({ + highlightPostTag: '__/ais-highlight__', + highlightPreTag: '__ais-highlight__', + }); // test if widget is not rendered yet at this point expect(rendering.callCount).toBe(0); @@ -136,4 +139,56 @@ describe('connectInfiniteHits', () => { expect(fourthRenderingOptions.hits).toEqual(thirdHits); expect(fourthRenderingOptions.results).toEqual(thirdResults); }); + + it('escape highlight properties if requested', () => { + const rendering = sinon.stub(); + const makeWidget = connectInfiniteHits(rendering); + const widget = makeWidget({escapeHits: true}); + + const helper = jsHelper(fakeClient, '', {}); + helper.search = sinon.stub(); + + widget.init({ + helper, + state: helper.state, + createURL: () => '#', + onHistoryChange: () => {}, + }); + + const firstRenderingOptions = rendering.lastCall.args[0]; + expect(firstRenderingOptions.hits).toEqual([]); + expect(firstRenderingOptions.results).toBe(undefined); + + const hits = [ + { + _highlightResult: { + foobar: { + value: '', + }, + }, + }, + ]; + + const results = new SearchResults(helper.state, [{hits}]); + widget.render({ + results, + state: helper.state, + helper, + createURL: () => '#', + }); + + const escapedHits = [ + { + _highlightResult: { + foobar: { + value: '<script>foobar</script>', + }, + }, + }, + ]; + + const secondRenderingOptions = rendering.lastCall.args[0]; + expect(secondRenderingOptions.hits).toEqual(escapedHits); + expect(secondRenderingOptions.results).toEqual(results); + }); }); diff --git a/src/connectors/infinite-hits/connectInfiniteHits.js b/src/connectors/infinite-hits/connectInfiniteHits.js index 7a1b3e7191..e42c426dd3 100644 --- a/src/connectors/infinite-hits/connectInfiniteHits.js +++ b/src/connectors/infinite-hits/connectInfiniteHits.js @@ -1,3 +1,4 @@ +import escapeHits, {tagConfig} from '../../lib/escape-highlight.js'; import {checkRendering} from '../../lib/utils.js'; const usage = `Usage: @@ -12,7 +13,9 @@ var customInfiniteHits = connectInfiniteHits(function render(params, isFirstRend // } }); search.addWidget( - customInfiniteHits() + customInfiniteHits({ + escapeHits: true, + }) ); Full documentation available at https://community.algolia.com/instantsearch.js/connectors/connectInfiniteHits.html `; @@ -26,13 +29,18 @@ Full documentation available at https://community.algolia.com/instantsearch.js/c * @property {Object} widgetParams All original widget options forwarded to the `renderFn`. */ +/** + * @typedef {Object} CustomInfiniteHitsWidgetOptions + * @property {boolean} [escapeHits = false] If true, escape HTML tags from `hits[i]._highlightResult`. + */ + /** * **InfiniteHits** connector provides the logic to create custom widgets that will render an continuous list of results retrieved from Algolia. * * This connector provides a `InfiniteHitsRenderingOptions.showMore()` function to load next page of matched results. * @type {Connector} * @param {function(InfiniteHitsRenderingOptions, boolean)} renderFn Rendering function for the custom **InfiniteHits** widget. - * @return {function(object)} Re-usable widget factory for a custom **InfiniteHits** widget. + * @return {function(CustomInfiniteHitsWidgetOptions)} Re-usable widget factory for a custom **InfiniteHits** widget. * @example * // custom `renderFn` to render the custom InfiniteHits widget * function renderFn(InfiniteHitsRenderingOptions, isFirstRendering) { @@ -73,6 +81,10 @@ export default function connectInfiniteHits(renderFn) { const getShowMore = helper => () => helper.nextPage().search(); return { + getConfiguration() { + return tagConfig; + }, + init({instantSearchInstance, helper}) { this.showMore = getShowMore(helper); @@ -91,6 +103,10 @@ export default function connectInfiniteHits(renderFn) { hitsCache = []; } + if (widgetParams.escapeHits && results.hits && results.hits.length > 0) { + results.hits = escapeHits(results.hits); + } + hitsCache = [...hitsCache, ...results.hits]; const isLastPage = results.nbPages <= results.page + 1; diff --git a/src/lib/__tests__/escape-highlight-test.js b/src/lib/__tests__/escape-highlight-test.js new file mode 100644 index 0000000000..b14d3d99ec --- /dev/null +++ b/src/lib/__tests__/escape-highlight-test.js @@ -0,0 +1,136 @@ +import escapeHits from '../escape-highlight'; + +describe('escapeHits()', () => { + it('should escape highlighProperty simple text value', () => { + const hits = [{ + _snippetResult: { + foobar: { + value: '', + }, + }, + _highlightResult: { + foobar: { + value: '', + }, + }, + }]; + + expect(escapeHits(hits)).toEqual([{ + _highlightResult: { + foobar: { + value: '<script>foobar</script>', + }, + }, + _snippetResult: { + foobar: { + value: '<script>foobar</script>', + }, + }, + }]); + }); + + it('should escape highlighProperty nested object value', () => { + const hits = [{ + _highlightResult: { + foobar: { + value: { + foo: '', + bar: '', + }, + }, + }, + _snippetResult: { + foobar: { + value: { + foo: '', + bar: '', + }, + }, + }, + }]; + + expect(escapeHits(hits)).toEqual([{ + _highlightResult: { + foobar: { + value: { + foo: '<script>bar</script>', + bar: '<script>foo</script>', + }, + }, + }, + _snippetResult: { + foobar: { + value: { + foo: '<script>bar</script>', + bar: '<script>foo</script>', + }, + }, + }, + }]); + }); + + it('should escape highlighProperty array of string as value', () => { + const hits = [{ + _highlightResult: { + foobar: { + value: [ + '', + '', + ], + }, + }, + _snippetResult: { + foobar: { + value: [ + '', + '', + ], + }, + }, + }]; + + expect(escapeHits(hits)).toEqual([{ + _highlightResult: { + foobar: { + value: [ + '<script>bar</script>', + '<script>foo</script>', + ], + }, + }, + _snippetResult: { + foobar: { + value: [ + '<script>bar</script>', + '<script>foo</script>', + ], + }, + }, + }]); + }); + + it('should not escape twice the same results', () => { + const hits = [{ + _highlightResult: { + foobar: { + value: '', + }, + }, + }]; + + escapeHits(hits); + escapeHits(hits); + + const output = [{ + _highlightResult: { + foobar: { + value: '<script>foo</script>', + }, + }, + }]; + + output.__escaped = true; + + expect(hits).toEqual(output); + }); +}); diff --git a/src/lib/escape-highlight.js b/src/lib/escape-highlight.js new file mode 100644 index 0000000000..d7026dc148 --- /dev/null +++ b/src/lib/escape-highlight.js @@ -0,0 +1,54 @@ +import reduce from 'lodash/reduce'; +import escape from 'lodash/escape'; +import isPlainObject from 'lodash/isPlainObject'; +import isArray from 'lodash/isArray'; +import mapValues from 'lodash/mapValues'; + +export const tagConfig = { + highlightPreTag: '__ais-highlight__', + highlightPostTag: '__/ais-highlight__', +}; + +function replaceWithEmAndEscape(value) { + return escape(value) + .replace(new RegExp(tagConfig.highlightPreTag, 'g'), '') + .replace(new RegExp(tagConfig.highlightPostTag, 'g'), ''); +} + +function recursiveEscape(input) { + return reduce(input, (output, value, key) => { + if (typeof value.value === 'string') { + value.value = replaceWithEmAndEscape(value.value); + } + + if (isPlainObject(value.value)) { + value.value = mapValues(value.value, replaceWithEmAndEscape); + } + + if (isArray(value.value)) { + value.value = value.value.map(replaceWithEmAndEscape); + } + + return {...output, [key]: value}; + }, {}); +} + +export default function escapeHits(hits) { + if (hits.__escaped === undefined) { + hits.__escaped = true; + + return hits.map(hit => { + if (hit._highlightResult) { + hit._highlightResult = recursiveEscape(hit._highlightResult); + } + + if (hit._snippetResult) { + hit._snippetResult = recursiveEscape(hit._snippetResult); + } + + return hit; + }); + } + + return hits; +} diff --git a/src/widgets/hits/hits.js b/src/widgets/hits/hits.js index cd184abc51..fa57fe95dc 100644 --- a/src/widgets/hits/hits.js +++ b/src/widgets/hits/hits.js @@ -82,6 +82,7 @@ hits({ * @property {HitsTemplates} [templates] Templates to use for the widget. * @property {HitsTransforms} [transformData] Method to change the object passed to the templates. * @property {HitsCSSClasses} [cssClasses] CSS classes to add. + * @property {boolean} [escapeHits = false] Escape HTML entities from hits string values. */ /** @@ -101,7 +102,8 @@ hits({ * empty: 'No results', * item: 'Hit {{objectID}}: {{{_highlightResult.name.value}}}' * }, - * hitsPerPage: 6 + * hitsPerPage: 6, + * escapeHits: true, * }) * ); */ @@ -110,6 +112,7 @@ export default function hits({ cssClasses: userCssClasses = {}, templates = defaultTemplates, transformData, + escapeHits = false, }) { if (!container) { throw new Error(`Must provide a container.${usage}`); @@ -136,7 +139,7 @@ export default function hits({ try { const makeHits = connectHits(specializedRenderer); - return makeHits(); + return makeHits({escapeHits}); } catch (e) { throw new Error(usage); } diff --git a/src/widgets/infinite-hits/__tests__/infinite-hits-test.js b/src/widgets/infinite-hits/__tests__/infinite-hits-test.js index 17985cc210..e7ebd86e85 100644 --- a/src/widgets/infinite-hits/__tests__/infinite-hits-test.js +++ b/src/widgets/infinite-hits/__tests__/infinite-hits-test.js @@ -42,8 +42,11 @@ describe('infiniteHits()', () => { results = {hits: [{first: 'hit', second: 'hit'}]}; }); - it('Does not have a specific configuration', () => { - expect(widget.getConfiguration).toBe(undefined); + it('It does have a specific configuration', () => { + expect(widget.getConfiguration()).toEqual({ + highlightPostTag: '__/ais-highlight__', + highlightPreTag: '__ais-highlight__', + }); }); it('calls twice ReactDOM.render(, container)', () => { @@ -72,10 +75,10 @@ describe('infiniteHits()', () => { }); expect(ReactDOM.render.calledTwice).toBe(true, 'ReactDOM.render called twice'); - const propsWithIsLastPageFalse = {...(getProps({...results, page: 0, nbPages: 2})), isLastPage: false}; + const propsWithIsLastPageFalse = {...getProps({...results, page: 0, nbPages: 2}), isLastPage: false}; expect(ReactDOM.render.firstCall.args[0]).toEqualJSX(); expect(ReactDOM.render.firstCall.args[1]).toEqual(container); - const propsWithIsLastPageTrue = {...(getProps({...results, page: 1, nbPages: 2})), isLastPage: true}; + const propsWithIsLastPageTrue = {...getProps({...results, page: 1, nbPages: 2}), isLastPage: true}; expect(ReactDOM.render.secondCall.args[0]).toEqualJSX(); expect(ReactDOM.render.secondCall.args[1]).toEqual(container); }); diff --git a/src/widgets/infinite-hits/infinite-hits.js b/src/widgets/infinite-hits/infinite-hits.js index 4d6337677b..65e211bb92 100644 --- a/src/widgets/infinite-hits/infinite-hits.js +++ b/src/widgets/infinite-hits/infinite-hits.js @@ -56,6 +56,7 @@ const usage = ` Usage: infiniteHits({ container, + [ escapeHits = false ], [ showMoreLabel ], [ cssClasses.{root,empty,item}={} ], [ templates.{empty,item} | templates.{empty} ], @@ -88,6 +89,7 @@ infiniteHits({ * @property {string} [showMoreLabel="Show more results"] label used on the show more button. * @property {InfiniteHitsTransforms} [transformData] Method to change the object passed to the templates. * @property {InfiniteHitsCSSClasses} [cssClasses] CSS classes to add. + * @property {boolean} [escapeHits = false] Escape HTML entities from hits string values. */ /** @@ -107,7 +109,8 @@ infiniteHits({ * empty: 'No results', * item: 'Hit {{objectID}}: {{{_highlightResult.name.value}}}' * }, - * hitsPerPage: 3 + * hitsPerPage: 3, + * escapeHits: true, * }) * ); */ @@ -117,6 +120,7 @@ export default function infiniteHits({ showMoreLabel = 'Show more results', templates = defaultTemplates, transformData, + escapeHits = false, } = {}) { if (!container) { throw new Error(`Must provide a container.${usage}`); @@ -141,7 +145,7 @@ export default function infiniteHits({ try { const makeInfiniteHits = connectInfiniteHits(specializedRenderer); - return makeInfiniteHits(); + return makeInfiniteHits({escapeHits}); } catch (e) { throw new Error(usage); }