karma-runner / karma

Spectacular Test Runner for JavaScript
http://karma-runner.github.io
MIT License
11.96k stars 1.71k forks source link

The value of window.__karma__.files is not properly escaped #1876

Open Joris-van-der-Wel opened 8 years ago

Joris-van-der-Wel commented 8 years ago

One of the files being served in my project has a single quote in its name. Karma is unable to run any tests because of this.

This is the culprit: https://github.com/karma-runner/karma/blob/19dd8249c6ce692004f4d52f8c0b2bb863e1f3fb/lib/middleware/karma.js#L148

Why not use JSON.stringify()?

Moumi commented 8 years ago

It seems that this issue can be solved for single quotes, but for double quotes... Well, you should never create double quotes in a filename. So, I worked on a fix and test for this and will provide a PR this evening for it.

Joris-van-der-Wel commented 8 years ago

Would it not be easier to use:

return util.format("  %s: '%s'", JSON.stringify(filePath), file.sha)

This way you can not make any mistake in the escaping.

console.log(JSON.stringify('foo\n"bar"')) // "foo\n\"bar\""
Moumi commented 8 years ago

There are some cases that when a character is escaped the loading of the file would go wrong. The most safe way would be to find the cases that are 'common' and escape only those characters.

Joris-van-der-Wel commented 8 years ago

How so?

All that we are doing is passing a JavaScript variable from one context (node) to another (the browser), JSON.stringify should take care of that. If something starts failing with that, you probably discovered a different issue.

alancutter commented 8 years ago

I'm still hitting this problem in the latest release: https://github.com/karma-runner/karma/releases/tag/v0.13.22 The merged change doesn't seem to be included for some reason. https://github.com/karma-runner/karma/blob/v0.13.22/lib/middleware/karma.js#L139