Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: EE-1.8.2.GA_P04
-
Fix Version/s: EE-1.8.2.GA_P06
-
Labels:None
-
Environment:Seam
-
Assignee Priority:P1
-
Salesforce Case Reference:
Description
<redirect view-id="/xhtml/loginator.xhtml">
<param name="t1" value="hellorvnworld" />
<param name="t2" value="RkqF3RW4QZzhF0JrlznFr6d99LOIuMW52h3Vp1dPH8R0007XuIsIgoXjaSCuGD6OIOvECogMjt8=" />
<param name="t3" value="0000000rvn11111111111" />
<param name="t4" value="rvn" />
<param name="t5" value="RVN" />
<param name="t6" value="RkqF3RW4QZzhF0JrlznFr6d99LOIuMW52h3Vp1dPH8Rvn7XuIsIgoXjaSCuGD6OIOvECogMjt8=" />
</redirect>
In this example, a parameter with name “t1” gets lost because its value is “hellorvnworld”, which contains “rvn” as a substring. Parameters t1, t3, and t4 will be null when trying to read the http request parameter after redirect.
This appears to be due to the following code at com.icesoft.util.SeamUtilities:194:
while(st.hasMoreTokens() ){
token = st.nextToken();
if ( (token.indexOf(conversationIdParameter) == -1) &&
(token.indexOf(conversationParentParameter) == -1) &&
token.indexOf("rvn") == -1 ) {
tokenList.add( token );
}
}
which indiscriminately discards request parameters if “rvn” is found anywhere in the value.
Note, this will also apply to the string “cid” when used as the conversationIdParameter.
Activity
- All
- Comments
- History
- Activity
- Remote Attachments
- Subversion
We discussed this in the core meeting and decided that, since the current code has been there for years and doing the namespace solution is likely overkill, that we would simply perform more strict checking of the parameters.
So in this case we are changing the logic to check strictly for "rvn" (case-sensitive) and not just any parameter that contains the substring "rvn". The same is done for Seam's conversation id but since that is configurable we can't just do it statically.
The bottom line is that "rvn" should be considered a reserved parameter name along with the Seam conversation id but that most other parameters should work fine.
I don't have a Seam based test case to check this against. Arran, do you have something I could use?
Checked in fix. Can reopen if testing shows any ill-effects.
Yes, the problem appears to be fairly obvious. Should I assume there is no test case for this?
The quickest/easiest way to fix this is to make the token checking more strict so that it's not just searching for request parameters that contain the keys "rvn" and "cid". However, it won't fix the scenarios where "rvn" or "cid" are used directly as parameters. In those cases, the parameters will need to be considered reserved and not allowed as parameter names by the application developer.
As a more complete solution, we could namespace the parameter key (e.g. ice.rvn) but I'm not sure of the wider effect that might have on the framework.