Skip to content
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

Not encoding as base64 breaks setups with xml escaping #172

Closed
dxlbnl opened this issue Jan 22, 2020 · 6 comments
Closed

Not encoding as base64 breaks setups with xml escaping #172

dxlbnl opened this issue Jan 22, 2020 · 6 comments

Comments

@dxlbnl
Copy link

dxlbnl commented Jan 22, 2020

  • Rollup Plugin Name: @rollup/plugin-image
  • Rollup Plugin Version: 2.0.1
  • Rollup Version: 1.29.x
  • Operating System (or Browser): Brave/FF
  • Node Version: -

How Do We Reproduce?

As it's quite involved setting up a svelte environment in a repl.it, I dont' have the time to do this atm. MR #136 introduced new behaviour (This update broke my workflow.)
That behaviour breaks existing setups as it's introducing html entities to the produced output by the imported images, and that is a security concern for many environments (as it's a vector for XSRF vurnerabilities).

Expected Behavior

Images should always be base64 encoded

Actual Behavior

SVG images allow for XSRF, since html entities are not encoded.

@shellscape
Copy link
Collaborator

If you are sure this is a bug, please take the time to refile this issue using the issue template.

Hey there, thanks for opening an issue - Unfortunately you missed (or may have disregarded) the message about issue templates being required. Because our issue template was removed, we assume that this is a support request, and that's not something we offer here. Also, if you refile, please don't ignore Issue sections. They're in the template because they're useful to the maintainers of this project.

@dxlbnl
Copy link
Author

dxlbnl commented Jan 22, 2020

@shellscape Sorry, was unaware there was anything useful in the tempalte. I updated the issue, could you reopen it

@shellscape
Copy link
Collaborator

@neoel this is frustrating for maintainers. The clip below is highlighted quite prominently in the issue template.

Please - do not - remove this template.
Please - do not - skip or remove parts of this template.
Or your issue may be closed.

While I appreciate you replacing the removed template, you've removed parts of it and disregarded the instructions in the template, which pretty much nukes why we have it there in the first place. Most notably:

  Issues without minimal reproductions will be closed! Please provide one by:
  1. Using the REPL.it plugin reproduction template at https://repl.it/@rollup/rollup-plugin-repro
  2. Provide a minimal repository link (Read https://git.io/fNzHA for instructions).
     These may take more time to triage than the other options.
  3. Using the REPL at https://rollupjs.org/repl/, or

There can be a leaning curve when first starting to work with open source projects, but knowing how to interact and report issues properly on Github projects is a good skill to hone. We'll be happy to reopen the issue when you've taken the time to follow our template correctly.

@dxlbnl
Copy link
Author

dxlbnl commented Jan 22, 2020

@shellscape Creating a Repl for this case is not going to be minimal, also this bug does not really fit your template nicely. I've updated it to explain the problems it has. Hope this is sufficient.

@shellscape
Copy link
Collaborator

Sorry, we don't have the bandwidth to triage issues without reproductions.

@dxlbnl
Copy link
Author

dxlbnl commented Jan 22, 2020

NVM then as #173 hopefully fixes the bug. This bug is accepted in a #136 btw, where the author and reviewer weren't aware of the breakages (as indicated by the template), as with the security concerns of allowing plain html in a src tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants