You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacques Le Roux <ja...@les7arts.com> on 2017/11/23 18:44:10 UTC
Re: svn commit: r1805677 - in /ofbiz:
branches/release16.11/framework/webapp/src/main/java/org/apache/ofbiz/webap
p/control/ContextFilter.java
ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webap
p/control/ContextFilter.java
Le 23/11/2017 à 19:22, Jacques Le Roux a écrit :
> Le 23/11/2017 à 17:16, Jacques Le Roux a écrit :
>> Hi Deepak, All,
>>
>> Deepak, after OFBIZ-9537, OFBIZ-9653, OFBIZ-9914 I have used your suggestion to change the documentation in site-conf.xsd at r1816167
>>
>> We need now to check that the request-redirect which use no redirect-param attribute are doing what they are supposed to do...
> Like everyone of us I'm lazy (are not programmer lazy by nature) when it comes to tedious tasks like this and there should not be any major issues,
> so let it be.
>
>> Else, even if it works, I believe they should be changed to request-redirect-no-param to avoid future confusion
> Done at revision: 1816180
There was a typo in r1816180, fixed at revision: 1816183 for OFBIZ-9997
It's request-redirect-noparam not request-redirect-no-param ;)
Jacques
>
> Jacques
>
>>
>> Jacques
>>
>>
>> Le 22/08/2017 à 09:21, Deepak Dixit a écrit :
>>> I re-read code again, and we did not removed _REQ_ATTR_MAP_ related code
>>> from RequestHandler.java, I think we are fine with it.
>>>
>>> Now we need to make sure that we used request-redirect properly in
>>> controller.xml
>>> IMO following is the use cases for response type:
>>>
>>> - request: All request param available in next request
>>> - request-redirect: Redirect only mentioned param in controller.xml, using
>>> redirect-param attrribute
>>> We need to fix the controller entries that uses request-redirect
>>> without specifying redirect-param.
>>> - request-redirect-no-param: only redirect to new request without passing
>>> any previous param.
>>>
>>> Here we need to check how we pass the system generated request attribute,
>>> like
>>> - error/success message list
>>> - FORWARDED_FROM_SERVLET etc.
>>>
>>> Thanks & Regards
>>> --
>>> Deepak Dixit
>>> www.hotwaxsystems.com
>>> www.hotwax.co
>>>
>>> On Tue, Aug 22, 2017 at 11:46 AM, Deepak Dixit <
>>> deepak.dixit@hotwaxsystems.com> wrote:
>>>
>>>> Hi Jacques,
>>>>
>>>> We need to keep the ContextFilter code with condition,
>>>> https://issues.apache.org/jira/browse/OFBIZ-9537
>>>>
>>>> Thanks & Regards
>>>> --
>>>> Deepak Dixit
>>>> www.hotwaxsystems.com
>>>> www.hotwax.co
>>>>
>>>> On Tue, Aug 22, 2017 at 2:16 AM, <jl...@apache.org> wrote:
>>>>
>>>>> Author: jleroux
>>>>> Date: Mon Aug 21 20:46:28 2017
>>>>> New Revision: 1805677
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1805677&view=rev
>>>>> Log:
>>>>> Fixed: Ecommerce login/logout don't work properly for trunk and stable
>>>>> (OFBIZ-9240)
>>>>>
>>>>> ecomseo trunk and stable (R16)
>>>>>
>>>>> Get to https://demo-trunk.ofbiz.apache.org/ecomseo/
>>>>> or https://demo-stable.ofbiz.apache.org/ecomseo/
>>>>> login => main page logged in
>>>>> logout => main page, not logged in
>>>>> login => main page logged in
>>>>> Use the "Not you" link => 404
>>>>> Refresh (F5 key) get you to the main page, not logged in
>>>>>
>>>>> So it works almost correctly but you need a refresh (F5 key) for the "Not
>>>>> you"
>>>>> link, not sure why yet.
>>>>> It"s the same locally with OFBIZ-9206 fixed*, w/ or w/o portoffset
>>>>> ecommerce
>>>>> trunk and stable (R16)
>>>>>
>>>>> Get to https://ofbiz-vm2.apache.org:8443/ecommerce
>>>>> or https://ofbiz-vm2.apache.org:18443/ecommerce
>>>>> login => blank page (no 404 in access log)
>>>>> Refresh (F5 key) get you to the main page, logged in
>>>>> logout => blank page (no 404 in access log)
>>>>> Refresh (F5 key) get you to the main page, not logged in
>>>>> login => blank page (no 404 in access log)
>>>>> Refresh (F5 key) get you to the main page, logged in
>>>>> use the "Not you" link => blank page (no 404 in access log)
>>>>> Refresh (F5 key) get you to the main page, not logged in
>>>>>
>>>>> So it works almost correctly but you need a refresh (F5 key) between in
>>>>> the
>>>>> 3 cases, not sure why yet.
>>>>> It's the same locally with OFBIZ-9206 fixed, w/ or w/o portoffset
>>>>>
>>>>> Rohit:
>>>>> The sequencing order of ControlFilter and ContextFilter is causing this
>>>>> problem.
>>>>> I tried to swap the order in case ecommerce but changing the sequence
>>>>> will not
>>>>> work because other filters(like ContentUrlFilter, CatalogUrlFilter)
>>>>> depends on
>>>>> the context prepared by ContextFilter.
>>>>>
>>>>> The issue is like when we are doing chaining of request using
>>>>> request-redirect
>>>>> than one request attribute(FORWARDED_FROM_SERVLET) set to TRUE from
>>>>> ControlServlet. In case of ecommerce ContextFilter run before
>>>>> ControlFilter
>>>>> which copies all the attribute from parent request to redirect request and
>>>>> when execution reaches the ControlFilter for the redirect request than
>>>>> request
>>>>> will fail to hit the servlet due to the code present
>>>>> at line 126(ControlFilter.java).
>>>>>
>>>>> To fix this problem I removed the code which copies all the attribute from
>>>>> parent request to redirect request because this thing is already taken
>>>>> care in
>>>>> the RequestHandler.
>>>>>
>>>>> Thanks: Rohit and Jacopo for the analysis and the fix
>>>>>
>>>>> Modified:
>>>>> ofbiz/branches/release16.11/framework/webapp/src/main/java/
>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java
>>>>> ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/
>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java
>>>>>
>>>>> Modified: ofbiz/branches/release16.11/framework/webapp/src/main/java/
>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/branches/release16.11/fra
>>>>> mework/webapp/src/main/java/org/apache/ofbiz/webapp/contro
>>>>> l/ContextFilter.java?rev=1805677&r1=1805676&r2=1805677&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- ofbiz/branches/release16.11/framework/webapp/src/main/java/
>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java (original)
>>>>> +++ ofbiz/branches/release16.11/framework/webapp/src/main/java/
>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java Mon Aug 21 20:46:28
>>>>> 2017
>>>>> @@ -115,19 +115,6 @@ public class ContextFilter implements Fi
>>>>> // set the server root url
>>>>> httpRequest.setAttribute("_SERVER_ROOT_URL_",
>>>>> UtilHttp.getServerRootUrl(httpRequest));
>>>>>
>>>>> - // request attributes from redirect call
>>>>> - String reqAttrMapHex = (String) httpRequest.getSession().getAt
>>>>> tribute("_REQ_ATTR_MAP_");
>>>>> - if (UtilValidate.isNotEmpty(reqAttrMapHex)) {
>>>>> - byte[] reqAttrMapBytes = StringUtil.fromHexString(reqAt
>>>>> trMapHex);
>>>>> - Map<String, Object> reqAttrMap =
>>>>> checkMap(UtilObject.getObject(reqAttrMapBytes), String.class,
>>>>> Object.class);
>>>>> - if (reqAttrMap != null) {
>>>>> - for (Map.Entry<String, Object> entry:
>>>>> reqAttrMap.entrySet()) {
>>>>> - httpRequest.setAttribute(entry.getKey(),
>>>>> entry.getValue());
>>>>> - }
>>>>> - }
>>>>> - httpRequest.getSession().removeAttribute("_REQ_ATTR_MAP_");
>>>>> - }
>>>>> -
>>>>> if (request.getCharacterEncoding() == null) {
>>>>> request.setCharacterEncoding(defaultCharacterEncoding);
>>>>> }
>>>>>
>>>>> Modified: ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/
>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/fra
>>>>> mework/webapp/src/main/java/org/apache/ofbiz/webapp/contro
>>>>> l/ContextFilter.java?rev=1805677&r1=1805676&r2=1805677&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/
>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java (original)
>>>>> +++ ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/
>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java Mon Aug 21 20:46:28
>>>>> 2017
>>>>> @@ -115,19 +115,6 @@ public class ContextFilter implements Fi
>>>>> // set the server root url
>>>>> httpRequest.setAttribute("_SERVER_ROOT_URL_",
>>>>> UtilHttp.getServerRootUrl(httpRequest));
>>>>>
>>>>> - // request attributes from redirect call
>>>>> - String reqAttrMapHex = (String) httpRequest.getSession().getAt
>>>>> tribute("_REQ_ATTR_MAP_");
>>>>> - if (UtilValidate.isNotEmpty(reqAttrMapHex)) {
>>>>> - byte[] reqAttrMapBytes = StringUtil.fromHexString(reqAt
>>>>> trMapHex);
>>>>> - Map<String, Object> reqAttrMap =
>>>>> checkMap(UtilObject.getObject(reqAttrMapBytes), String.class,
>>>>> Object.class);
>>>>> - if (reqAttrMap != null) {
>>>>> - for (Map.Entry<String, Object> entry:
>>>>> reqAttrMap.entrySet()) {
>>>>> - httpRequest.setAttribute(entry.getKey(),
>>>>> entry.getValue());
>>>>> - }
>>>>> - }
>>>>> - httpRequest.getSession().removeAttribute("_REQ_ATTR_MAP_");
>>>>> - }
>>>>> -
>>>>> if (request.getCharacterEncoding() == null) {
>>>>> request.setCharacterEncoding(defaultCharacterEncoding);
>>>>> }
>>>>>
>>>>>
>>>>>
>>
>
>
Re: svn commit: r1805677 - in /ofbiz: branches/release16.11/framework/webapp/src/main/java/org/apache/ofbiz/webap
p/control/ContextFilter.java ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webap
p/control/ContextFilter.java
Posted by Deepak Dixit <de...@hotwaxsystems.com>.
Thanks Jacques,
Thanks & Regards
--
Deepak Dixit
www.hotwaxsystems.com
www.hotwax.co
On Fri, Nov 24, 2017 at 12:14 AM, Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:
> Le 23/11/2017 à 19:22, Jacques Le Roux a écrit :
>
>> Le 23/11/2017 à 17:16, Jacques Le Roux a écrit :
>>
>>> Hi Deepak, All,
>>>
>>> Deepak, after OFBIZ-9537, OFBIZ-9653, OFBIZ-9914 I have used your
>>> suggestion to change the documentation in site-conf.xsd at r1816167
>>>
>>> We need now to check that the request-redirect which use no
>>> redirect-param attribute are doing what they are supposed to do...
>>>
>> Like everyone of us I'm lazy (are not programmer lazy by nature) when it
>> comes to tedious tasks like this and there should not be any major issues,
>> so let it be.
>>
>> Else, even if it works, I believe they should be changed to
>>> request-redirect-no-param to avoid future confusion
>>>
>> Done at revision: 1816180
>>
> There was a typo in r1816180, fixed at revision: 1816183 for OFBIZ-9997
> It's request-redirect-noparam not request-redirect-no-param ;)
>
> Jacques
>
>
>> Jacques
>>
>>
>>> Jacques
>>>
>>>
>>> Le 22/08/2017 à 09:21, Deepak Dixit a écrit :
>>>
>>>> I re-read code again, and we did not removed _REQ_ATTR_MAP_ related code
>>>> from RequestHandler.java, I think we are fine with it.
>>>>
>>>> Now we need to make sure that we used request-redirect properly in
>>>> controller.xml
>>>> IMO following is the use cases for response type:
>>>>
>>>> - request: All request param available in next request
>>>> - request-redirect: Redirect only mentioned param in controller.xml,
>>>> using
>>>> redirect-param attrribute
>>>> We need to fix the controller entries that uses request-redirect
>>>> without specifying redirect-param.
>>>> - request-redirect-no-param: only redirect to new request without
>>>> passing
>>>> any previous param.
>>>>
>>>> Here we need to check how we pass the system generated request
>>>> attribute,
>>>> like
>>>> - error/success message list
>>>> - FORWARDED_FROM_SERVLET etc.
>>>>
>>>> Thanks & Regards
>>>> --
>>>> Deepak Dixit
>>>> www.hotwaxsystems.com
>>>> www.hotwax.co
>>>>
>>>> On Tue, Aug 22, 2017 at 11:46 AM, Deepak Dixit <
>>>> deepak.dixit@hotwaxsystems.com> wrote:
>>>>
>>>> Hi Jacques,
>>>>>
>>>>> We need to keep the ContextFilter code with condition,
>>>>> https://issues.apache.org/jira/browse/OFBIZ-9537
>>>>>
>>>>> Thanks & Regards
>>>>> --
>>>>> Deepak Dixit
>>>>> www.hotwaxsystems.com
>>>>> www.hotwax.co
>>>>>
>>>>> On Tue, Aug 22, 2017 at 2:16 AM, <jl...@apache.org> wrote:
>>>>>
>>>>> Author: jleroux
>>>>>> Date: Mon Aug 21 20:46:28 2017
>>>>>> New Revision: 1805677
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1805677&view=rev
>>>>>> Log:
>>>>>> Fixed: Ecommerce login/logout don't work properly for trunk and stable
>>>>>> (OFBIZ-9240)
>>>>>>
>>>>>> ecomseo trunk and stable (R16)
>>>>>>
>>>>>> Get to https://demo-trunk.ofbiz.apache.org/ecomseo/
>>>>>> or https://demo-stable.ofbiz.apache.org/ecomseo/
>>>>>> login => main page logged in
>>>>>> logout => main page, not logged in
>>>>>> login => main page logged in
>>>>>> Use the "Not you" link => 404
>>>>>> Refresh (F5 key) get you to the main page, not logged in
>>>>>>
>>>>>> So it works almost correctly but you need a refresh (F5 key) for the
>>>>>> "Not
>>>>>> you"
>>>>>> link, not sure why yet.
>>>>>> It"s the same locally with OFBIZ-9206 fixed*, w/ or w/o portoffset
>>>>>> ecommerce
>>>>>> trunk and stable (R16)
>>>>>>
>>>>>> Get to https://ofbiz-vm2.apache.org:8443/ecommerce
>>>>>> or https://ofbiz-vm2.apache.org:18443/ecommerce
>>>>>> login => blank page (no 404 in access log)
>>>>>> Refresh (F5 key) get you to the main page, logged in
>>>>>> logout => blank page (no 404 in access log)
>>>>>> Refresh (F5 key) get you to the main page, not logged in
>>>>>> login => blank page (no 404 in access log)
>>>>>> Refresh (F5 key) get you to the main page, logged in
>>>>>> use the "Not you" link => blank page (no 404 in access log)
>>>>>> Refresh (F5 key) get you to the main page, not logged in
>>>>>>
>>>>>> So it works almost correctly but you need a refresh (F5 key) between
>>>>>> in
>>>>>> the
>>>>>> 3 cases, not sure why yet.
>>>>>> It's the same locally with OFBIZ-9206 fixed, w/ or w/o portoffset
>>>>>>
>>>>>> Rohit:
>>>>>> The sequencing order of ControlFilter and ContextFilter is causing
>>>>>> this
>>>>>> problem.
>>>>>> I tried to swap the order in case ecommerce but changing the sequence
>>>>>> will not
>>>>>> work because other filters(like ContentUrlFilter, CatalogUrlFilter)
>>>>>> depends on
>>>>>> the context prepared by ContextFilter.
>>>>>>
>>>>>> The issue is like when we are doing chaining of request using
>>>>>> request-redirect
>>>>>> than one request attribute(FORWARDED_FROM_SERVLET) set to TRUE from
>>>>>> ControlServlet. In case of ecommerce ContextFilter run before
>>>>>> ControlFilter
>>>>>> which copies all the attribute from parent request to redirect
>>>>>> request and
>>>>>> when execution reaches the ControlFilter for the redirect request than
>>>>>> request
>>>>>> will fail to hit the servlet due to the code present
>>>>>> at line 126(ControlFilter.java).
>>>>>>
>>>>>> To fix this problem I removed the code which copies all the attribute
>>>>>> from
>>>>>> parent request to redirect request because this thing is already taken
>>>>>> care in
>>>>>> the RequestHandler.
>>>>>>
>>>>>> Thanks: Rohit and Jacopo for the analysis and the fix
>>>>>>
>>>>>> Modified:
>>>>>> ofbiz/branches/release16.11/framework/webapp/src/main/java/
>>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java
>>>>>> ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/
>>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java
>>>>>>
>>>>>> Modified: ofbiz/branches/release16.11/framework/webapp/src/main/java/
>>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/branches/release16.11/fra
>>>>>> mework/webapp/src/main/java/org/apache/ofbiz/webapp/contro
>>>>>> l/ContextFilter.java?rev=1805677&r1=1805676&r2=1805677&view=diff
>>>>>> ============================================================
>>>>>> ==================
>>>>>> --- ofbiz/branches/release16.11/framework/webapp/src/main/java/
>>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java (original)
>>>>>> +++ ofbiz/branches/release16.11/framework/webapp/src/main/java/
>>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java Mon Aug 21
>>>>>> 20:46:28
>>>>>> 2017
>>>>>> @@ -115,19 +115,6 @@ public class ContextFilter implements Fi
>>>>>> // set the server root url
>>>>>> httpRequest.setAttribute("_SERVER_ROOT_URL_",
>>>>>> UtilHttp.getServerRootUrl(httpRequest));
>>>>>>
>>>>>> - // request attributes from redirect call
>>>>>> - String reqAttrMapHex = (String)
>>>>>> httpRequest.getSession().getAt
>>>>>> tribute("_REQ_ATTR_MAP_");
>>>>>> - if (UtilValidate.isNotEmpty(reqAttrMapHex)) {
>>>>>> - byte[] reqAttrMapBytes = StringUtil.fromHexString(reqAt
>>>>>> trMapHex);
>>>>>> - Map<String, Object> reqAttrMap =
>>>>>> checkMap(UtilObject.getObject(reqAttrMapBytes), String.class,
>>>>>> Object.class);
>>>>>> - if (reqAttrMap != null) {
>>>>>> - for (Map.Entry<String, Object> entry:
>>>>>> reqAttrMap.entrySet()) {
>>>>>> - httpRequest.setAttribute(entry.getKey(),
>>>>>> entry.getValue());
>>>>>> - }
>>>>>> - }
>>>>>> - httpRequest.getSession().removeAttribute("_REQ_ATTR_MAP_");
>>>>>> - }
>>>>>> -
>>>>>> if (request.getCharacterEncoding() == null) {
>>>>>> request.setCharacterEncoding(defaultCharacterEncoding);
>>>>>> }
>>>>>>
>>>>>> Modified: ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/
>>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/fra
>>>>>> mework/webapp/src/main/java/org/apache/ofbiz/webapp/contro
>>>>>> l/ContextFilter.java?rev=1805677&r1=1805676&r2=1805677&view=diff
>>>>>> ============================================================
>>>>>> ==================
>>>>>> --- ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/
>>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java (original)
>>>>>> +++ ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/
>>>>>> org/apache/ofbiz/webapp/control/ContextFilter.java Mon Aug 21
>>>>>> 20:46:28
>>>>>> 2017
>>>>>> @@ -115,19 +115,6 @@ public class ContextFilter implements Fi
>>>>>> // set the server root url
>>>>>> httpRequest.setAttribute("_SERVER_ROOT_URL_",
>>>>>> UtilHttp.getServerRootUrl(httpRequest));
>>>>>>
>>>>>> - // request attributes from redirect call
>>>>>> - String reqAttrMapHex = (String)
>>>>>> httpRequest.getSession().getAt
>>>>>> tribute("_REQ_ATTR_MAP_");
>>>>>> - if (UtilValidate.isNotEmpty(reqAttrMapHex)) {
>>>>>> - byte[] reqAttrMapBytes = StringUtil.fromHexString(reqAt
>>>>>> trMapHex);
>>>>>> - Map<String, Object> reqAttrMap =
>>>>>> checkMap(UtilObject.getObject(reqAttrMapBytes), String.class,
>>>>>> Object.class);
>>>>>> - if (reqAttrMap != null) {
>>>>>> - for (Map.Entry<String, Object> entry:
>>>>>> reqAttrMap.entrySet()) {
>>>>>> - httpRequest.setAttribute(entry.getKey(),
>>>>>> entry.getValue());
>>>>>> - }
>>>>>> - }
>>>>>> - httpRequest.getSession().removeAttribute("_REQ_ATTR_MAP_");
>>>>>> - }
>>>>>> -
>>>>>> if (request.getCharacterEncoding() == null) {
>>>>>> request.setCharacterEncoding(defaultCharacterEncoding);
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>
>>
>>
>