Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-5847

If define the & and combine with "part" that encode to ∂

    Details

      Description

      XML widget problems: If define the & and combine with "part" that encode to ∂
      Example >>>
      BEFORE: viewprofile?status=Y&partyId=Demo
      AFTER: viewprofile?status=Y∂yId=Demo

      1. OFBiz WorkEffort Manager Calendar.png
        199 kB
        Supachai Chaima-ngua (Tor)
      2. OFBIZ-5847.patch
        14 kB
        Gil Portenseigne

        Activity

        Hide
        Francis Douet Francis Douet added a comment -

        Example can be found here: http://demo-stable-ofbiz.apache.org/partymgr/control/viewprofile?partyId=Company
        In the List Related Contacts screenlet, Create New link.

        Show
        Francis Douet Francis Douet added a comment - Example can be found here: http://demo-stable-ofbiz.apache.org/partymgr/control/viewprofile?partyId=Company In the List Related Contacts screenlet, Create New link.
        Hide
        soledad Nicolas Malin added a comment -

        Hi Supachai, Francis,

        The reason is that the parameters with present directly on the target value. It's better to use parameter element to set your values.

        You can see the correction below :

        -            <link target="${parameters._LAST_VIEW_NAME_}?portalPageId=${parameters.portalPageId}&amp;partyId=${parameters.partyId}&amp;editPartyRel=Y"/>
        +            <link target="${parameters._LAST_VIEW_NAME_}">
        +                <parameter param-name="partyId"/>
        +                <parameter param-name="portalPageId"/>
        +                <parameter param-name="editPartyRel" value="Y"/>
        +            </link>
        
        Show
        soledad Nicolas Malin added a comment - Hi Supachai, Francis, The reason is that the parameters with present directly on the target value. It's better to use parameter element to set your values. You can see the correction below : - <link target= "${parameters._LAST_VIEW_NAME_}?portalPageId=${parameters.portalPageId}&amp;partyId=${parameters.partyId}&amp;editPartyRel=Y" /> + <link target= "${parameters._LAST_VIEW_NAME_}" > + <parameter param-name= "partyId" /> + <parameter param-name= "portalPageId" /> + <parameter param-name= "editPartyRel" value= "Y" /> + </link>
        Hide
        soledad Nicolas Malin added a comment -

        Thanks to detect the error in List Related Contacts screenlet, correction commited on release :

        • 1637716 for trunk
        • 1637717 for 13.07
        • 1637719 for 12.04
        Show
        soledad Nicolas Malin added a comment - Thanks to detect the error in List Related Contacts screenlet, correction commited on release : 1637716 for trunk 1637717 for 13.07 1637719 for 12.04
        Hide
        tortechnocom Supachai Chaima-ngua (Tor) added a comment -

        That solution is fine. Thank you.

        Show
        tortechnocom Supachai Chaima-ngua (Tor) added a comment - That solution is fine. Thank you.
        Hide
        utcb Leon added a comment - - edited

        I have a question. The valid entity of '∂' should be "&part;", why the "&part" without tailing ';' is replaced here?

        Show
        utcb Leon added a comment - - edited I have a question. The valid entity of '∂' should be "&part;", why the "&part" without tailing ';' is replaced here?
        Hide
        tortechnocom Supachai Chaima-ngua (Tor) added a comment - - edited

        I have got a bug on workeffort component (See an attach file). Please someone have a look.

        Show
        tortechnocom Supachai Chaima-ngua (Tor) added a comment - - edited I have got a bug on workeffort component (See an attach file). Please someone have a look.
        Hide
        soledad Nicolas Malin added a comment -

        Thanks Supachai, I confirm the error, and I can't indicate you to use parameter because we are on ftl .
        So I re-open a this jira. We need to found where the encoding failed !

        Show
        soledad Nicolas Malin added a comment - Thanks Supachai, I confirm the error, and I can't indicate you to use parameter because we are on ftl . So I re-open a this jira. We need to found where the encoding failed !
        Hide
        gil portenseigne Gil Portenseigne added a comment -

        After looking at where the encoding fails (without finding an evident solution) :
        In framework/widget/src/org/ofbiz/widget/WidgetWorker.java l.75 canonicalize method.

        I found in the patch provided, that the menu is an xml one, so can be fix the same way as said upper.

        The menu is also used in humanRes, i check and it seems to need no parameters, so no side-effect detected.

        Inside the ftl, the links are working with the parameters, so it's good

        Show
        gil portenseigne Gil Portenseigne added a comment - After looking at where the encoding fails (without finding an evident solution) : In framework/widget/src/org/ofbiz/widget/WidgetWorker.java l.75 canonicalize method. I found in the patch provided, that the menu is an xml one, so can be fix the same way as said upper. The menu is also used in humanRes, i check and it seems to need no parameters, so no side-effect detected. Inside the ftl, the links are working with the parameters, so it's good
        Hide
        soledad Nicolas Malin added a comment -

        Thanks Gil for this patch !

        It's commited on :

        • r1641096 for trunk
        • r1641097 for 13.07
        Show
        soledad Nicolas Malin added a comment - Thanks Gil for this patch ! It's commited on : r1641096 for trunk r1641097 for 13.07
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Still remains Leon's good question

        I have a question. The valid entity of '∂' should be "∂", why the "&part" without tailing ';' is replaced here?

        Have we an idea where this happened? Are we sure there could not be other side effects?

        Show
        jacques.le.roux Jacques Le Roux added a comment - Still remains Leon's good question I have a question. The valid entity of '∂' should be "∂", why the "&part" without tailing ';' is replaced here? Have we an idea where this happened? Are we sure there could not be other side effects?
        Hide
        gil portenseigne Gil Portenseigne added a comment -

        To ease the inquiry, it happens here :
        In framework/widget/src/org/ofbiz/widget/WidgetWorker.java l.75 canonicalize method. :

        localRequestName = StringUtil.defaultWebEncoder.canonicalize(localRequestName);

        Where defaultWebEncoder initialized in StringUtil :
        List<Codec> codecList = Arrays.asList(new HTMLEntityCodec(), new PercentCodec());
        defaultWebEncoder = new DefaultEncoder(codecList);

        The codec HTMLEntityCodec made it happen.

        Show
        gil portenseigne Gil Portenseigne added a comment - To ease the inquiry, it happens here : In framework/widget/src/org/ofbiz/widget/WidgetWorker.java l.75 canonicalize method. : localRequestName = StringUtil.defaultWebEncoder.canonicalize(localRequestName); Where defaultWebEncoder initialized in StringUtil : List<Codec> codecList = Arrays.asList(new HTMLEntityCodec(), new PercentCodec()); defaultWebEncoder = new DefaultEncoder(codecList); The codec HTMLEntityCodec made it happen.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks Gil, this reminds me we need to update ESAPI OFBIZ-5795, hopefully this will fix that.

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks Gil, this reminds me we need to update ESAPI OFBIZ-5795 , hopefully this will fix that.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        It seems the ESAPI update fixed this last issue Leon asked for

        Show
        jacques.le.roux Jacques Le Roux added a comment - It seems the ESAPI update fixed this last issue Leon asked for
        Hide
        utcb Leon added a comment -

        Hi, Jacques,

        I have test it with new ESAPI (2.1), but the problem still occurs.

        Seems ESAPI treats the html entity without trailing semicolon same as with that.

        See http://owasp-esapi-java.googlecode.com/svn/trunk_doc/1.4.4/org/owasp/esapi/reference/DefaultEncoder.html#canonicalize(java.lang.String, it's doc for 1.4.4, however the related source does not change more in new release. There's a note like "Note that all of these formats may possibly render properly in a browser without the trailing semicolon."

        Show
        utcb Leon added a comment - Hi, Jacques, I have test it with new ESAPI (2.1), but the problem still occurs. Seems ESAPI treats the html entity without trailing semicolon same as with that. See http://owasp-esapi-java.googlecode.com/svn/trunk_doc/1.4.4/org/owasp/esapi/reference/DefaultEncoder.html#canonicalize(java.lang.String , it's doc for 1.4.4, however the related source does not change more in new release. There's a note like "Note that all of these formats may possibly render properly in a browser without the trailing semicolon."
        Hide
        jacques.le.roux Jacques Le Roux added a comment - - edited

        Indeed the issue comes from the ESAPI lib, when we use GET style URL parameters in screens/forms links instead of POST style as Nicolas fixed 3 cases.

        I made a review, we have 51 `target*&` occurences OOTB

        • The <form ... target > links are not concerned (see edit budget item for instance)
        • Nor the <hyperlink target> links (see systems notes for instance)
        • Nor <hyperlink target> links (see ListProductStoreFacility, but not in trunk due to OFBIZ-6051)
        • Nor <on-event-update-area area-target> links (see ListProductStoreFacility EditProductStoreFacility)

        So it seems only the <link target> links are concerned and moreover hopefully maybe only in menus. We have no longer any of them OOTB. So at least OFBiz is ok .

        I will close this issue, this can no lnoger appear in new and custom code, because the new ESAPI implemtation now throws a

        org.ofbiz.base.util.UtilCodec$IntrusionException: Input validation failure
        

        in such cases (jus try to revert r1637716 in trunk)

        Happy end

        Show
        jacques.le.roux Jacques Le Roux added a comment - - edited Indeed the issue comes from the ESAPI lib, when we use GET style URL parameters in screens/forms links instead of POST style as Nicolas fixed 3 cases. I made a review, we have 51 `target*&` occurences OOTB The <form ... target > links are not concerned (see edit budget item for instance) Nor the <hyperlink target> links (see systems notes for instance) Nor <hyperlink target> links (see ListProductStoreFacility, but not in trunk due to OFBIZ-6051 ) Nor <on-event-update-area area-target> links (see ListProductStoreFacility EditProductStoreFacility) So it seems only the <link target> links are concerned and moreover hopefully maybe only in menus. We have no longer any of them OOTB. So at least OFBiz is ok . I will close this issue, this can no lnoger appear in new and custom code, because the new ESAPI implemtation now throws a org.ofbiz.base.util.UtilCodec$IntrusionException: Input validation failure in such cases (jus try to revert r1637716 in trunk) Happy end
        Hide
        pfm.smits Pierre Smits added a comment - - edited

        Maybe this issue is (also) a good moment to rethink how we render linking within menu items, screens, screenlets and forms... See: OFBIZ-6034

        So that we have a solution that works consistently wherever it is applied.

        Show
        pfm.smits Pierre Smits added a comment - - edited Maybe this issue is (also) a good moment to rethink how we render linking within menu items, screens, screenlets and forms... See: OFBIZ-6034 So that we have a solution that works consistently wherever it is applied.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Yes that's right Pierre, at least it's worth to be investigated

        Show
        jacques.le.roux Jacques Le Roux added a comment - Yes that's right Pierre, at least it's worth to be investigated

          People

          • Assignee:
            soledad Nicolas Malin
            Reporter:
            tortechnocom Supachai Chaima-ngua (Tor)
          • Votes:
            1 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development