[DELTASPIKE-1074] Severe problem in setUrlParam() (windowhandler.js)

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 1.5.3
• Fix Version/s: 1.5.4
• Component/s: JSF-Module
• Assignee: Thomas Andraschko
• Reporter: Sean Flanigan
• Created: 08/Feb/16 00:23
• Updated: 05/Apr/16 07:44
• Resolved: 17/Feb/16 15:18

Description

I noticed a potential problem with the URI manipulation while reviewing https://github.com/apache/deltaspike/commit/323c7d38e9d949385eb9d90c47e8971548ab5ed4 :

> a.href = uri.replace('?&', '?').replace('&&', '&');

I think it's only safe to coalesce multiple '&'s in the query part (after '?), not in the path path (before '?'). In the path part, '&'s are just characters, not parameter delimiters.

Also, it's possible that the URI could be like this:
?&&&dswid=1234&&&
with more than two ampersands in a row, perhaps at the beginning of the query string.

(Attn: tandraschko)

EDIT: It looks like this commit has totally broken setUrlParam(), because dswh.utils.generateRequestToken() generates requestToken as a number, but setUrlParam() now assumes that parameterValue is a string, and tries to call replace() on it. This leads to errors like this:

http://localhost:49178/zanata/javax.faces.resource/deltaspike/windowhandler.js.seam 10:413 Uncaught TypeError: b.replace is not a function

I think this wasn't discovered in earlier testing because windowhandler.js seems to be aggressively cached. I had some trouble convincing Chrome to reload windowhandler.js.

If possible, I would suggest including a hash value in the URL generated by <ds:windowId/> and windowhandler.html, eg something like <script type="text/javascript" src="/javax.faces.resource/windowhandler.js?hash=871623874619283469"></script>, where the hash is generated from the contents of windowhandler.js, to allow aggressive caching without risk of missing changes.