gatsby-image: noscriptImg does not escape attribute values

ekuiter commented 4 years ago

Description

In https://github.com/gatsbyjs/gatsby/blob/3666917d7218433e0ae0adde6bd024e358e83ba0/packages/gatsby-image/src/index.js#L233, the noscriptImg function (which is responsible for displaying images when JavaScript is disabled) uses string interpolation to inject the passed props:

https://github.com/gatsbyjs/gatsby/blob/3666917d7218433e0ae0adde6bd024e358e83ba0/packages/gatsby-image/src/index.js#L239

This fails whenever one of the props contains double quotes, or more precisely, it causes the browser to fail parsing such attributes.

Steps to reproduce

Create two Imgs as follows:

<Img fluid={...] title="MyTitle" />
<Img fluid={...] title={'"MyTitle"'} />

Run gatsby build, view in a browser and disable JavaScript. In the delivered HTML, we have ...

<picture><source srcset="..." sizes="..." srcset="..." alt="" title="MyTitle" style="..."></picture>
<picture><source srcset="..." sizes="..." srcset="..." alt="" title=""MyTitle"" style="..."></picture>

... which will obviously fail to parse in the browser. Firefox, for example, interprets the second Img as follows in the DOM:

<picture><source srcset="..." sizes="..." srcset="..." alt="" title="" mytitle""="" style="..."></picture>

Expected result

Props given to Img, such as title in the example above, should be properly escaped. For example, we could have title=""MyTitle"" in the delivered HTML instead.

Actual result

Browsers trip over all affected attributes. In my case (latest Firefox), the attribute is silently ignored.

Environment

  System:
    OS: Linux 4.4 Ubuntu 18.04.1 LTS (Bionic Beaver)
    CPU: (8) x64 Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz
    Shell: 4.4.19 - /bin/bash
  Binaries:
    Node: 8.10.0 - /usr/bin/node
    Yarn: 1.13.0 - /mnt/c/Users/Elias/AppData/Roaming/npm/yarn
    npm: 6.14.2 - /usr/local/bin/npm
  Languages:
    Python: 2.7.15 - /usr/bin/python
  npmPackages:
    gatsby: ^2.19.30 => 2.19.30
    gatsby-image: ^2.2.42 => 2.2.42
    gatsby-plugin-google-analytics: ^2.1.36 => 2.1.36
    gatsby-plugin-manifest: ^2.2.42 => 2.2.42
    gatsby-plugin-react-helmet: ^3.1.22 => 3.1.22
    gatsby-plugin-sharp: ^2.4.5 => 2.4.5
    gatsby-plugin-typography: ^2.3.22 => 2.3.22
    gatsby-source-filesystem: ^2.1.48 => 2.1.48
    gatsby-transformer-sharp: ^2.3.16 => 2.3.16

vladar commented 4 years ago

Hey @ekuiter !

Thanks for creating this issue. I can confirm it - minimal reproduction is as simple as creating a new site from blog starter and adding title (with a " character) to Image component inside bio.js.

github-actions[bot] commented 4 years ago

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open! As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

ascorbic commented 3 years ago

This is fixed in gatsby-plugin-image

gatsbyjs / gatsby