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

Style CSP error when using options.legend.useHTML=true (style-src 'self') #7633

Closed
codemasterover9000 opened this issue Jan 9, 2018 · 13 comments
Closed

Comments

@codemasterover9000
Copy link

@codemasterover9000 codemasterover9000 commented Jan 9, 2018

We are using styled mode but when using a custom HTML based legend item. We still get CSP errors caused by the the highcharts library.

This is caused at highcharts.src.js:6585 (version 6.0.4) in the SVGRenderer.html() method at the following code wrapper.css({position: 'absolute'}).

Using wrapper.style.position = 'absolute' instead of would probably fix the problem.

@KacperMadej
Copy link
Contributor

@KacperMadej KacperMadej commented Jan 10, 2018

Hi @codemasterover9000

Thank you for reporting about the problem.
Please post a JSFiddle demo of the problem, so we could double check that not only a problem is resolved but also your problem will be solved.

@codemasterover9000
Copy link
Author

@codemasterover9000 codemasterover9000 commented Jan 11, 2018

We use this configuration: http://jsfiddle.net/g9yj6aqh/
The legend with useHTML seems to cause the problem. setting useHTML to false causes the CSP error to disappear.

@sebastianbochan
Copy link
Contributor

@sebastianbochan sebastianbochan commented Jan 11, 2018

Hi @codemasterover9000
When I run the demo: http://jsfiddle.net/3qe6v2y9/ (styled mode) I always have inline styles (with or withoout useHTML). Could you confirm that I have the right scenario?

@codemasterover9000
Copy link
Author

@codemasterover9000 codemasterover9000 commented Jan 11, 2018

I found a solution by modifying the follwing code in SvgRenderer.js here to the following:

                    if (isMS && !svg) {
                        css(this.element, styles);
                    } else {
                        hyphenate = function(a, b) {
                            return '-' + b.toLowerCase();
                        };
                        var resultStyles = {};
                        objectEach(styles, function(style, n) {
                            if (inArray(n, svgPseudoProps) === -1) {
                                resultStyles[n.replace(/([A-Z])/g, hyphenate)] = style;
                            }
                        });
                        css(elem, resultStyles);
                    }
@codemasterover9000
Copy link
Author

@codemasterover9000 codemasterover9000 commented Jan 12, 2018

When I remove useHTML for the legend I dont get CSP errors originating from highcharts when I run your fiddle. When I set useHTML to true I get multiple CSP errors originating from Legend.renderItem(). Using the code in my previous comment fixes this issue.

The real point of this issue imo is that the library should never set the style attribute directly (unsafe-line) in any case. But modify its properties directly (which in theory would also perform better).

In our case this would have saved us a lot of time as we are implementing a CSP where unsafe-inline is not allowed. Now we need to switch al our code to styled mode. Which is a better way to style but not the thing we wanted to spend our resources on atm ;)

@KacperMadej
Copy link
Contributor

@KacperMadej KacperMadej commented Jan 15, 2018

@codemasterover9000 Styled mode shouldn't generate inline CSS - if it does, please post a simple demo for it. Classic mode doesn't have this restriction, so please make sure that you are using correct code version for your requirements.

@codemasterover9000
Copy link
Author

@codemasterover9000 codemasterover9000 commented Jan 15, 2018

Fiddle: http://jsfiddle.net/g9yj6aqh/1/ same code as the one I posted before but with the styled mode js/css files (Just saw I added the classic mode js in the previous fiddle).

I also gave you a code pointer where you can clearly see the style attribute being set. This code should not even exist in the styled mode js files according to your comment. Maybe take some time to look at the comment/source next time. I am also spending my time to help you fix this...

@KacperMadej
Copy link
Contributor

@KacperMadej KacperMadej commented Jan 15, 2018

Thank you for your help, time and the live demo.

Currently there's no way around this because style is used to position elements.

Internal note: inline CSS for styled mode extending issue #6173

@codemasterover9000
Copy link
Author

@codemasterover9000 codemasterover9000 commented Jan 16, 2018

Yes thats why I proposed a fix where style attributes are set in a secure way 5 days ago. This at least fixes this case (and probably others).

@pawelfus
Copy link
Contributor

@pawelfus pawelfus commented Jan 16, 2018

@codemasterover9000
Copy link
Author

@codemasterover9000 codemasterover9000 commented Jan 18, 2018

Not sure if I should open a new issue but I found a similar issue in exporting.src.js here

@TorsteinHonsi
Copy link
Collaborator

@TorsteinHonsi TorsteinHonsi commented Jan 24, 2018

I think the real purpose of the snippet above is to handle styles separately for SVG elements and HTML elements. Will the following version resolve your case?

			// serialize and set style attribute
			if (elem.namespaceURI === this.SVG_NS) {
				hyphenate = function (a, b) {
					return '-' + b.toLowerCase();
				};
				objectEach(styles, function (style, n) {
					if (inArray(n, svgPseudoProps) === -1) {
						serializedCss +=
						n.replace(/([A-Z])/g, hyphenate) + ':' +
						style + ';';
					}
				});
				if (serializedCss) {
					attr(elem, 'style', serializedCss); // #1881
				}
			} else {
				css(elem, styles);
			}
@codemasterover9000
Copy link
Author

@codemasterover9000 codemasterover9000 commented Jan 25, 2018

@TorsteinHonsi Yes the above code resolves the issue.

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

Successfully merging a pull request may close this issue.

None yet
5 participants