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

Deferring or avoiding ImageRenderer #71

Closed
anthonyryan1 opened this Issue May 13, 2017 · 4 comments

Comments

2 participants
@anthonyryan1
Copy link
Contributor

anthonyryan1 commented May 13, 2017

I recently upgraded from an older version of this (qr.js) and noticed that the new version generated a Content Security Policy warning the website where I deployed it. The website doesn't allow data URIs under any circumstances and the ImageRenderer creates an image tag and populates it with a data URL as part of the constructor now.

One solution might be to not render the image until a method call requesting it has been made.

@neocotic

This comment has been minimized.

Copy link
Owner

neocotic commented May 29, 2017

@anthonyryan1 I'm a bit confused, to be honest. Why don't you simply modify your CSP header? I believe that img-src 'self' data: should allow what you're looking for.

As for the last part, this is already what happens. QRious does not render anything until you instantiate QRious. Also, why would deferring the image rendering workaround CSP?

@anthonyryan1

This comment has been minimized.

Copy link
Contributor

anthonyryan1 commented May 29, 2017

That is possible, but data URIs are generally frowned upon within CSP since it's impossible to determine the "origin" of a data URI (as compared to a normal URI where you can use CORS rules and compare to a domain whitelist).

I'm using QRius to print a QR code on a canvas. I'm not using an img tag and I'm not calling any methods (eg: toDataURL) that I thought would generate an image. The violation occurs as soon as any img tag has a data URL attached to it as the source, so in QRius it happens here: https://github.com/neocotic/qrious/blob/master/src/renderer/ImageRenderer.js#L39

I'm happy to submit a patch for this, but I wanted to talk about any reasons the code works the way it does first, in case there's something I'm missing.

My current thought would be that if a canvas element is passed to QRius, we could simply use the CanvasRenderer. But if an img element is passed, both renderers will need to be run, since the canvas is necessary for producing the image.

Does this sound like a good approach to you?

@neocotic

This comment has been minimized.

Copy link
Owner

neocotic commented May 29, 2017

This makes more sense to me now and I like your approach, however, we'd also need to continue to ensure that image is rendered, if needed. It'd also need to support the rendering of image any time after it has been referenced as it could well have been inserted into the page. We could place the image property behind a getter method to track any access to it.

This sounds like a worthwhile change, but I'll need to consider it further to see if we can achieve it while retaining backwards compatibility.

@neocotic neocotic added this to the 2.3.0 milestone May 29, 2017

@neocotic neocotic referenced this issue May 30, 2017

Merged

Release 2.3.0 #78

5 of 5 tasks complete

neocotic added a commit that referenced this issue May 31, 2017

@neocotic

This comment has been minimized.

Copy link
Owner

neocotic commented May 31, 2017

These changes will be included in the 2.3.0 release which I'm hoping to push out today (within the next hour probably - see #78). Please let me know how you get on with this.

@neocotic neocotic closed this May 31, 2017

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