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

[BUG] data urls are not handled properly #18

Closed
isaacs opened this issue Oct 18, 2021 · 0 comments · Fixed by #19
Closed

[BUG] data urls are not handled properly #18

isaacs opened this issue Oct 18, 2021 · 0 comments · Fixed by #19

Comments

@isaacs
Copy link
Contributor

isaacs commented Oct 18, 2021

What / Why

data: urls should be supported the "same" as fetched URLs, in the sense that they may be aborted, and should support any valid data: urls, whether base64 encoded or not.

const c = new AbortController()
fetch('data:text/plain;base64,SGVsbG8sIFdvcmxkIQ==', { signal: c.signal })
  .then(r => console.log(r), e => console.error(e))
c.abort()

Expect it to fail with an AbortError. Instead it works. (This mirrors node-fetch, but diverges from browser fetch.)

const urls = [
  'data:,Hello%2C%20World%21',
  'data:text/html,%3Ch1%3EHello%2C%20World%21%3C%2Fh1%3E',
  'data:text/html,<script>alert('hi');</script>',
]
Promise.all(urls.map(u => fetch(u)).then(() => console.log('it worked'))

Expect: load all data urls. Actual: fails because they are not base64 encoded.

Again, mirrors node-fetch, but not browser fetch.

isaacs added a commit that referenced this issue Nov 8, 2021
1. No longer requires `;base64` to be included on `data:` URIs.
2. Supports immediately aborting `data:` URIs.

Fix: #18
isaacs added a commit that referenced this issue Nov 8, 2021
1. No longer requires `;base64` to be included on `data:` URIs.
2. Supports immediately aborting `data:` URIs.

Fix: #18
isaacs added a commit that referenced this issue Nov 8, 2021
1. No longer requires `;base64` to be included on `data:` URIs.
2. Supports immediately aborting `data:` URIs.

Fix: #18
isaacs added a commit that referenced this issue Nov 9, 2021
1. No longer requires `;base64` to be included on `data:` URIs.
2. Supports immediately aborting `data:` URIs.

Fix: #18
wraithgar pushed a commit that referenced this issue Mar 1, 2022
1. No longer requires `;base64` to be included on `data:` URIs.
2. Supports immediately aborting `data:` URIs.

Fix: #18
wraithgar pushed a commit that referenced this issue Mar 1, 2022
1. No longer requires `;base64` to be included on `data:` URIs.
2. Supports immediately aborting `data:` URIs.

Fix: #18
wraithgar pushed a commit that referenced this issue Mar 1, 2022
1. No longer requires `;base64` to be included on `data:` URIs.
2. Supports immediately aborting `data:` URIs.

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

Successfully merging a pull request may close this issue.

1 participant