Skip to content

Commit

Permalink
Proposed fix for WW-5029 for the 2.5.x branch (apache#347)
Browse files Browse the repository at this point in the history
* Proposed fix for WW-5029 for the 2.5.x branch:
- NOTE: If the PR is accepted please credit Maxime Clement for this change as they found
        the issue, identified the probable cause/related details and opened the JIRA.
- Updated XWorkConfigurationProvider buildAllowedMethods(), loadGlobalAllowedMethods() so that
  they now handle situations when a SAX parser produces multiple elements to represent the tag
  body value.
- No changes to unit tests.

* Update commit to fix weakness identified by Maxime Clement:
- Implementation should now properly concatenate the node children values together (as a single unified string)
  in both buildAllowedMethods(), loadGlobalAllowedMethods() - before generating the method Set to be added.
- Made some eligible variables final.

* Update commit to provide new unit tests:
- Added unit tests to confirm the fixes for buildAllowedMethods(), loadGlobalAllowedMethods()
- Added Mock DOM classes sufficient for these tests.
- Added unit tests to cover buildResults() and loadGlobalResults().

(cherry picked from commit fb38a91)
  • Loading branch information
JCgH4164838Gh792C124B5 authored and yasserzamani committed Apr 20, 2019
1 parent f0776ae commit 47a8a21
Show file tree
Hide file tree
Showing 2 changed files with 831 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -874,15 +874,22 @@ protected Set<String> buildAllowedMethods(Element element, PackageConfig.Builder
if (allowedMethodsEls.getLength() > 0) {
// user defined 'allowed-methods' so used them whatever Strict DMI was enabled or not
allowedMethods = new HashSet<>(packageContext.getGlobalAllowedMethods());

if (allowedMethodsEls.getLength() > 0) {
Node n = allowedMethodsEls.item(0).getFirstChild();
if (n != null) {
String s = n.getNodeValue().trim();
if (s.length() > 0) {
allowedMethods.addAll(TextParseUtil.commaDelimitedStringToSet(s));
// Fix for WW-5029 (concatenate all possible text node children)
final Node allowedMethodsNode = allowedMethodsEls.item(0);
if (allowedMethodsNode != null) {
final NodeList allowedMethodsChildren = allowedMethodsNode.getChildNodes();
final StringBuilder allowedMethodsSB = new StringBuilder();
for (int i = 0; i < allowedMethodsChildren.getLength(); i++) {
Node allowedMethodsChildNode = allowedMethodsChildren.item(i);
String childNodeValue = (allowedMethodsChildNode != null ? allowedMethodsChildNode.getNodeValue() : "");
childNodeValue = (childNodeValue != null ? childNodeValue.trim() : "");
if (childNodeValue.length() > 0) {
allowedMethodsSB.append(childNodeValue);
}
}
if (allowedMethodsSB.length() > 0) {
allowedMethods.addAll(TextParseUtil.commaDelimitedStringToSet(allowedMethodsSB.toString()));
}
}
} else if (packageContext.isStrictMethodInvocation()) {
// user enabled Strict DMI but didn't defined action specific 'allowed-methods' so we use 'global-allowed-methods' only
Expand Down Expand Up @@ -937,11 +944,21 @@ protected void loadGlobalAllowedMethods(PackageConfig.Builder packageContext, El

if (globalAllowedMethodsElms.getLength() > 0) {
Set<String> globalAllowedMethods = new HashSet<>();
Node n = globalAllowedMethodsElms.item(0).getFirstChild();
if (n != null) {
String s = n.getNodeValue().trim();
if (s.length() > 0) {
globalAllowedMethods = TextParseUtil.commaDelimitedStringToSet(s);
// Fix for WW-5029 (concatenate all possible text node children)
Node globaAllowedMethodsNode = globalAllowedMethodsElms.item(0);
if (globaAllowedMethodsNode != null) {
NodeList globaAllowedMethodsChildren = globaAllowedMethodsNode.getChildNodes();
final StringBuilder globalAllowedMethodsSB = new StringBuilder();
for (int i = 0; i < globaAllowedMethodsChildren.getLength(); i++) {
Node globalAllowedMethodsChildNode = globaAllowedMethodsChildren.item(i);
String childNodeValue = (globalAllowedMethodsChildNode != null ? globalAllowedMethodsChildNode.getNodeValue() : "");
childNodeValue = (childNodeValue != null ? childNodeValue.trim() : "");
if (childNodeValue.length() > 0) {
globalAllowedMethodsSB.append(childNodeValue);
}
}
if (globalAllowedMethodsSB.length() > 0) {
globalAllowedMethods.addAll(TextParseUtil.commaDelimitedStringToSet(globalAllowedMethodsSB.toString()));
}
}
packageContext.addGlobalAllowedMethods(globalAllowedMethods);
Expand Down
Loading

0 comments on commit 47a8a21

Please sign in to comment.