From 25681cf5f67934818315ff8f6c2535cf6873c2f3 Mon Sep 17 00:00:00 2001 From: ilja Date: Sun, 18 Aug 2024 15:48:22 +0200 Subject: [PATCH] Don't require `#` in the data-mfm-color attribute For colour in MFM attributes, we expected a `#`, but that's apparently wrong. The BE translates the `color` attribute in `$[fg.color=000 text]` into `data-mfm-color=000`. But for the SCSS to work, we need to put it in the style attribute as `--mfm-color: #000`. Generally we just add the attribute value as-is in the `style` attribute, but now we have a special exception for color so we add a `#` before the value. --- src/components/rich_content/rich_content.jsx | 7 +-- .../specs/components/rich_content.spec.js | 43 ++++++++++++++++--- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/components/rich_content/rich_content.jsx b/src/components/rich_content/rich_content.jsx index 73feb294..21e0a021 100644 --- a/src/components/rich_content/rich_content.jsx +++ b/src/components/rich_content/rich_content.jsx @@ -194,12 +194,13 @@ export default { // Turn data-mfm- attributes into a string for the `style` attribute // If they have a value different than `true`, they need to be added to `style` // e.g. `attrs={'data-mfm-some': '1deg', 'data-mfm-thing': '5s'}` => "--mfm-some: 1deg;--mfm-thing: 5s;" - // Note that we only add the value to `style` when they contain only letters, numbers, dot, hash, or plus or minus signs + // Note that we only add the value to `style` when they contain only letters, numbers, dot, or minus signs // At the moment of writing, this should be enough for legitimite purposes and reduces the chance of injection by using special characters + // There is a special case for the `color` value, who is provided without `#`, but requires this in the `style` attribute let mfm_style = Object.keys(attrs).filter( - (key) => key.startsWith('data-mfm-') && attrs[key] !== true && /^[a-zA-Z0-9.\-+#]*$/.test(attrs[key]) + (key) => key.startsWith('data-mfm-') && attrs[key] !== true && /^[a-zA-Z0-9.\-]*$/.test(attrs[key]) ).map( - (key) => '--mfm-' + key.substr(9) + ': ' + attrs[key] + ';' + (key) => '--mfm-' + key.substr(9) + (key === 'data-mfm-color' ? ': #' : ': ') + attrs[key] + ';' ).reduce((a,v) => a+v, '') if (mfm_style !== '') { return [ diff --git a/test/unit/specs/components/rich_content.spec.js b/test/unit/specs/components/rich_content.spec.js index 4c01c1e1..2fd61367 100644 --- a/test/unit/specs/components/rich_content.spec.js +++ b/test/unit/specs/components/rich_content.spec.js @@ -40,9 +40,9 @@ describe('RichContent', () => { expect(wrapper.html().replace(/\n/g, '')).to.eql(compwrap(html)) }) - it('does not allow injection through MFM data- attributes', () => { - const html_ok = 'brrr' - const expected_ok = 'brrr' + it('it adds a # to the MFM color style value', () => { + const html_ok = 'this text is not white' + const expected_ok = 'this text is not white' const wrapper_ok = shallowMount(RichContent, { global, props: { @@ -53,20 +53,49 @@ describe('RichContent', () => { html: html_ok } }) - const html_nok = 'brrr' - const wrapper_nok = shallowMount(RichContent, { + + expect(wrapper_ok.html()).to.eql(compwrap(expected_ok)) + }) + + it('does not allow injection through MFM data- attributes', () => { + const html_ok = 'brrr' + const expected_ok = 'brrr' + const wrapper_ok = shallowMount(RichContent, { global, props: { attentions, handleLinks: true, greentext: true, emoji: [], - html: html_nok + html: html_ok + } + }) + const html_nok1 = 'brrr' + const wrapper_nok1 = shallowMount(RichContent, { + global, + props: { + attentions, + handleLinks: true, + greentext: true, + emoji: [], + html: html_nok1 + } + }) + const html_nok2 = 'brrr' + const wrapper_nok2 = shallowMount(RichContent, { + global, + props: { + attentions, + handleLinks: true, + greentext: true, + emoji: [], + html: html_nok2 } }) expect(wrapper_ok.html()).to.eql(compwrap(expected_ok)) - expect(wrapper_nok.html()).to.eql(compwrap(html_nok)) + expect(wrapper_nok1.html()).to.eql(compwrap(html_nok1)) + expect(wrapper_nok2.html()).to.eql(compwrap(html_nok2)) }) it('unescapes everything as needed', () => {