Error.ftl everywhere

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Error.ftl everywhere

Jacques Le Roux
Administrator
Hi,

While working on OFBIZ-11840 I thought about the solution I used for "[CVE-2020-1943] Apache OFBiz XSS Vulnerability"

So I tried that:

diff --git framework/common/webcommon/WEB-INF/common-controller.xml framework/common/webcommon/WEB-INF/common-controller.xml
index e6f9394cd4..9291cdbece 100644
--- framework/common/webcommon/WEB-INF/common-controller.xml
+++ framework/common/webcommon/WEB-INF/common-controller.xml
@@ -338,7 +338,7 @@ under the License.
      <!--========================== AJAX events =====================-->

      <!-- View Mappings -->
-    <view-map name="error" page="/error/error.jsp"/>
+    <view-map name="error" type="ftl" page="component://common/webcommon/error/Error.ftl"/>
      <view-map name="main" type="none"/>
      <view-map name="login" type="screen" page="component://common/widget/CommonScreens.xml#login"/>
      <view-map name="impersonated" type="screen" page="component://common/widget/CommonScreens.xml#impersonated"/>
diff --git framework/common/webcommon/WEB-INF/handlers-controller.xml framework/common/webcommon/WEB-INF/handlers-controller.xml
index be21b19fd9..1622d10ead 100644
--- framework/common/webcommon/WEB-INF/handlers-controller.xml
+++ framework/common/webcommon/WEB-INF/handlers-controller.xml
@@ -42,4 +42,5 @@ under the License.
      <handler name="screenfop" type="view" class="org.apache.ofbiz.widget.renderer.fo.ScreenFopViewHandler"/>
      <handler name="jsp" type="view" class="org.apache.ofbiz.webapp.view.JspViewHandler"/>
      <handler name="http" type="view" class="org.apache.ofbiz.webapp.view.HttpViewHandler"/>
+    <handler name="ftl" type="view" class="org.apache.ofbiz.webapp.ftl.FreeMarkerViewHandler"/>
  </site-conf>

It does not fix the OFBIZ-11840 issue but it works. I mean it correctly replaces error.jsp by error.ftl.

Few questions:

 1. Why having the ftl handlers only in webtools controller? BTW it makes the XSD documentation awkward because it speaks about the ftl handlers being
    in handlers-controller.xml
 2. Why not using error.ftl in common-controller.xml instead of error.jsp?
 3. Same question for plugins.

I believe we could change all that and definitely get rid of error.jsp (error.ftl is already in all supported releases branches)

What do you think?

Jacques

Reply | Threaded
Open this post in threaded view
|

Re: Error.ftl everywhere

Jacques Le Roux
Administrator

Le 05/07/2020 à 16:50, Jacques Le Roux a écrit :

> Hi,
>
> While working on OFBIZ-11840 I thought about the solution I used for "[CVE-2020-1943] Apache OFBiz XSS Vulnerability"
>
> So I tried that:
>
> diff --git framework/common/webcommon/WEB-INF/common-controller.xml framework/common/webcommon/WEB-INF/common-controller.xml
> index e6f9394cd4..9291cdbece 100644
> --- framework/common/webcommon/WEB-INF/common-controller.xml
> +++ framework/common/webcommon/WEB-INF/common-controller.xml
> @@ -338,7 +338,7 @@ under the License.
>      <!--========================== AJAX events =====================-->
>
>      <!-- View Mappings -->
> -    <view-map name="error" page="/error/error.jsp"/>
> +    <view-map name="error" type="ftl" page="component://common/webcommon/error/Error.ftl"/>
>      <view-map name="main" type="none"/>
>      <view-map name="login" type="screen" page="component://common/widget/CommonScreens.xml#login"/>
>      <view-map name="impersonated" type="screen" page="component://common/widget/CommonScreens.xml#impersonated"/>
> diff --git framework/common/webcommon/WEB-INF/handlers-controller.xml framework/common/webcommon/WEB-INF/handlers-controller.xml
> index be21b19fd9..1622d10ead 100644
> --- framework/common/webcommon/WEB-INF/handlers-controller.xml
> +++ framework/common/webcommon/WEB-INF/handlers-controller.xml
> @@ -42,4 +42,5 @@ under the License.
>      <handler name="screenfop" type="view" class="org.apache.ofbiz.widget.renderer.fo.ScreenFopViewHandler"/>
>      <handler name="jsp" type="view" class="org.apache.ofbiz.webapp.view.JspViewHandler"/>
>      <handler name="http" type="view" class="org.apache.ofbiz.webapp.view.HttpViewHandler"/>
> +    <handler name="ftl" type="view" class="org.apache.ofbiz.webapp.ftl.FreeMarkerViewHandler"/>
>  </site-conf>
>
> It does not fix the OFBIZ-11840 issue but it works. I mean it correctly replaces error.jsp by error.ftl.
>
> Few questions:
>
> 1. Why having the ftl handlers only in webtools controller? BTW it makes the XSD documentation awkward because it speaks about the ftl handlers being
>    in handlers-controller.xml
> 2. Why not using error.ftl in common-controller.xml instead of error.jsp?
> 3. Same question for plugins.
>
> I believe we could change all that and definitely get rid of error.jsp (error.ftl is already in all supported releases branches)
>
> What do you think?
>
> Jacques
>
Done with OFBIZ-11890

Jacques

Reply | Threaded
Open this post in threaded view
|

Re: Error.ftl everywhere

Nicolas Malin-2
Thanks Jacques,

You finished the work to remove the jsp error :)

Nicolas

On 15/07/2020 21:09, Jacques Le Roux wrote:

>
> Le 05/07/2020 à 16:50, Jacques Le Roux a écrit :
>> Hi,
>>
>> While working on OFBIZ-11840 I thought about the solution I used for
>> "[CVE-2020-1943] Apache OFBiz XSS Vulnerability"
>>
>> So I tried that:
>>
>> diff --git framework/common/webcommon/WEB-INF/common-controller.xml
>> framework/common/webcommon/WEB-INF/common-controller.xml
>> index e6f9394cd4..9291cdbece 100644
>> --- framework/common/webcommon/WEB-INF/common-controller.xml
>> +++ framework/common/webcommon/WEB-INF/common-controller.xml
>> @@ -338,7 +338,7 @@ under the License.
>>      <!--========================== AJAX events =====================-->
>>
>>      <!-- View Mappings -->
>> -    <view-map name="error" page="/error/error.jsp"/>
>> +    <view-map name="error" type="ftl"
>> page="component://common/webcommon/error/Error.ftl"/>
>>      <view-map name="main" type="none"/>
>>      <view-map name="login" type="screen"
>> page="component://common/widget/CommonScreens.xml#login"/>
>>      <view-map name="impersonated" type="screen"
>> page="component://common/widget/CommonScreens.xml#impersonated"/>
>> diff --git framework/common/webcommon/WEB-INF/handlers-controller.xml
>> framework/common/webcommon/WEB-INF/handlers-controller.xml
>> index be21b19fd9..1622d10ead 100644
>> --- framework/common/webcommon/WEB-INF/handlers-controller.xml
>> +++ framework/common/webcommon/WEB-INF/handlers-controller.xml
>> @@ -42,4 +42,5 @@ under the License.
>>      <handler name="screenfop" type="view"
>> class="org.apache.ofbiz.widget.renderer.fo.ScreenFopViewHandler"/>
>>      <handler name="jsp" type="view"
>> class="org.apache.ofbiz.webapp.view.JspViewHandler"/>
>>      <handler name="http" type="view"
>> class="org.apache.ofbiz.webapp.view.HttpViewHandler"/>
>> +    <handler name="ftl" type="view"
>> class="org.apache.ofbiz.webapp.ftl.FreeMarkerViewHandler"/>
>>  </site-conf>
>>
>> It does not fix the OFBIZ-11840 issue but it works. I mean it
>> correctly replaces error.jsp by error.ftl.
>>
>> Few questions:
>>
>> 1. Why having the ftl handlers only in webtools controller? BTW it
>> makes the XSD documentation awkward because it speaks about the ftl
>> handlers being
>>    in handlers-controller.xml
>> 2. Why not using error.ftl in common-controller.xml instead of
>> error.jsp?
>> 3. Same question for plugins.
>>
>> I believe we could change all that and definitely get rid of
>> error.jsp (error.ftl is already in all supported releases branches)
>>
>> What do you think?
>>
>> Jacques
>>
> Done with OFBIZ-11890
>
> Jacques
>

pEpkey.asc (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Error.ftl everywhere

Jacques Le Roux
Administrator
Hi Nicolas, All,

Last effort: I think we should now get rid of all error.jsp, error403.jsp and error404.jsp files and all references to these.

It's easy to remove commented out <error-page> references from 3 web.xml files (marketing, partymgr and workeffort). We are sure they are not used.

Then it makes sense to remove RequestHandler::getDefaultErrorPage and its only reference in ControlServlet::handle (since nothing exists in web.xml files)

Same for

    <<[tempName:'error.jsp', newName:'error.jsp', location:"webapp/${webappName}/error"],>>

in build.gradle.

Do I miss anything?

Thanks

Jacques

Le 16/07/2020 à 11:55, Nicolas Malin a écrit :

> Thanks Jacques,
>
> You finished the work to remove the jsp error :)
>
> Nicolas
>
> On 15/07/2020 21:09, Jacques Le Roux wrote:
>> Le 05/07/2020 à 16:50, Jacques Le Roux a écrit :
>>> Hi,
>>>
>>> While working on OFBIZ-11840 I thought about the solution I used for
>>> "[CVE-2020-1943] Apache OFBiz XSS Vulnerability"
>>>
>>> So I tried that:
>>>
>>> diff --git framework/common/webcommon/WEB-INF/common-controller.xml
>>> framework/common/webcommon/WEB-INF/common-controller.xml
>>> index e6f9394cd4..9291cdbece 100644
>>> --- framework/common/webcommon/WEB-INF/common-controller.xml
>>> +++ framework/common/webcommon/WEB-INF/common-controller.xml
>>> @@ -338,7 +338,7 @@ under the License.
>>>       <!--========================== AJAX events =====================-->
>>>
>>>       <!-- View Mappings -->
>>> -    <view-map name="error" page="/error/error.jsp"/>
>>> +    <view-map name="error" type="ftl"
>>> page="component://common/webcommon/error/Error.ftl"/>
>>>       <view-map name="main" type="none"/>
>>>       <view-map name="login" type="screen"
>>> page="component://common/widget/CommonScreens.xml#login"/>
>>>       <view-map name="impersonated" type="screen"
>>> page="component://common/widget/CommonScreens.xml#impersonated"/>
>>> diff --git framework/common/webcommon/WEB-INF/handlers-controller.xml
>>> framework/common/webcommon/WEB-INF/handlers-controller.xml
>>> index be21b19fd9..1622d10ead 100644
>>> --- framework/common/webcommon/WEB-INF/handlers-controller.xml
>>> +++ framework/common/webcommon/WEB-INF/handlers-controller.xml
>>> @@ -42,4 +42,5 @@ under the License.
>>>       <handler name="screenfop" type="view"
>>> class="org.apache.ofbiz.widget.renderer.fo.ScreenFopViewHandler"/>
>>>       <handler name="jsp" type="view"
>>> class="org.apache.ofbiz.webapp.view.JspViewHandler"/>
>>>       <handler name="http" type="view"
>>> class="org.apache.ofbiz.webapp.view.HttpViewHandler"/>
>>> +    <handler name="ftl" type="view"
>>> class="org.apache.ofbiz.webapp.ftl.FreeMarkerViewHandler"/>
>>>   </site-conf>
>>>
>>> It does not fix the OFBIZ-11840 issue but it works. I mean it
>>> correctly replaces error.jsp by error.ftl.
>>>
>>> Few questions:
>>>
>>> 1. Why having the ftl handlers only in webtools controller? BTW it
>>> makes the XSD documentation awkward because it speaks about the ftl
>>> handlers being
>>>     in handlers-controller.xml
>>> 2. Why not using error.ftl in common-controller.xml instead of
>>> error.jsp?
>>> 3. Same question for plugins.
>>>
>>> I believe we could change all that and definitely get rid of
>>> error.jsp (error.ftl is already in all supported releases branches)
>>>
>>> What do you think?
>>>
>>> Jacques
>>>
>> Done with OFBIZ-11890
>>
>> Jacques
>>
Reply | Threaded
Open this post in threaded view
|

Re: Error.ftl everywhere

Jacques Le Roux
Administrator
Le 16/07/2020 à 17:20, Jacques Le Roux a écrit :
>
> Then it makes sense to remove RequestHandler::getDefaultErrorPage and its only reference in ControlServlet::handle (since nothing exists in web.xml
> files)

I rather made error.ftl the new default

Done

Jacques