Page MenuHomePhabricator

Error for invalid on-wiki JavaScript can show wrong filename
Closed, ResolvedPublic

Description

When on-wiki JavaScript fails JSMin's (JSParser to be exact) validation, it makes the minified script file just throw an error. This error thrown includes the page name (e.g. User:Example/common.js).

This output is cached, which means if someone copies the JavaScript (e.g. to debug the problem), it will show the wrong filename/page name for that subsequent user.

This usually won't be too hard for a user to figure out (particularly if there's only one copy). However, the page name could be added to the cache key to fix this. I don't think this will have too much of a performance impact. There generally shouldn't be many identical copies of a script (there are certainly scripts widely imported, though).


Version: 1.22.0
Severity: enhancement

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 1:56 AM
bzimport set Reference to bz50919.
bzimport added a subscriber: Unknown Object (MLST).

This relates to ResourceLoaderModule::validateScriptFile which takes filename as argument but uses a hash of the script content for the cache key. Since the "parse error" message will reference e.g. "foo.js on line 5" it is cached along.

However when different users have the same script in their user space, it would potentially show the wrong filename.

Change 353590 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Simplify validateScriptFile() with getWithSetCallback

https://gerrit.wikimedia.org/r/353590

Change 353591 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Add filename to validateScriptFile cache key

https://gerrit.wikimedia.org/r/353591

Change 353590 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Simplify validateScriptFile() with getWithSetCallback

https://gerrit.wikimedia.org/r/353590

Change 353591 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Add filename to validateScriptFile cache key

https://gerrit.wikimedia.org/r/353591