[ 
https://issues.apache.org/jira/browse/OFBIZ-10030?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Michael Brohl updated OFBIZ-10030:
----------------------------------
    Description: 
In TrackingCodeEvents#checkTrackingCodeUrlParam, a provided tracking code is 
checked against the database. If not found, it returns "error".

{code:java}
            if (trackingCode == null) {
                Debug.logError("TrackingCode not found for trackingCodeId [" + 
trackingCodeId + "], ignoring this trackingCodeId.", module);
                //this return value will be ignored, but we'll designate this 
as an error anyway
                return "error";
            }
{code}

The notice "return value is ignored" is not true because in the event handler, 
a non-success return value is handled to throw an excpetion:


{code:java}
            // Invoke the pre-processor (but NOT in a chain)
            for (ConfigXMLReader.Event event: 
controllerConfig.getPreprocessorEventList().values()) {
                try {
                    String returnString = this.runEvent(request, response, 
event, null, "preprocessor");
                    if (returnString != null && 
!returnString.equalsIgnoreCase("success")) {
                        if (!returnString.contains(":_protect_:")) {
                            throw new EventHandlerException("Pre-Processor 
event [" + event.invoke + "] did not return 'success'.");
                        } else { // protect the view normally rendered and 
redirect to error response view
                            returnString = returnString.replace(":_protect_:", 
"");
                            if (returnString.length() > 0) {
                                request.setAttribute("_ERROR_MESSAGE_", 
returnString);
                            }
                            eventReturn = null;
                            // check to see if there is a "protect" response, 
if so it's ok else show the default_error_response_view
                            if 
(!requestMap.requestResponseMap.containsKey("protect")) {
                                String protectView = 
controllerConfig.getProtectView();
                                if (protectView != null) {
                                    overrideViewUri = protectView;
                                } else {
                                    overrideViewUri = 
UtilProperties.getPropertyValue("security.properties", 
"default.error.response.view");
                                    overrideViewUri = 
overrideViewUri.replace("view:", "");
                                    if ("none:".equals(overrideViewUri)) {
                                        interruptRequest = true;
                                    }
                                }
                            }
                        }
                    } else if (returnString == null) {
                        interruptRequest = true;
                    }
                } catch (EventHandlerException e) {
                    Debug.logError(e, module);
                }
            }
{code}

This leads to a flood of Exception stacktraces in the OFBiz logs.

In my opinion, the checkTrackingCodeUrlParam method should return "success" if 
the tracking code is not found and should log to loglevel "info". It's not an 
error.

What do others think?


  was:
In TrackingCodeEvents#checkTrackingCodeUrlParam, a provided tracking code is 
checked against the database. If not found, it returns "error".

{code:java}
            if (trackingCode == null) {
                Debug.logError("TrackingCode not found for trackingCodeId [" + 
trackingCodeId + "], ignoring this trackingCodeId.", module);
                //this return value will be ignored, but we'll designate this 
as an error anyway
                return "error";
            }
{code}

The notice "return value is ignored" is not true because in the event handler, 
a non-success return value is handled to throw an excpetion:


{code:java}
            // Invoke the pre-processor (but NOT in a chain)
            for (ConfigXMLReader.Event event: 
controllerConfig.getPreprocessorEventList().values()) {
                try {
                    String returnString = this.runEvent(request, response, 
event, null, "preprocessor");
                    if (returnString != null && 
!returnString.equalsIgnoreCase("success")) {
                        if (!returnString.contains(":_protect_:")) {
                            throw new EventHandlerException("Pre-Processor 
event [" + event.invoke + "] did not return 'success'.");
                        } else { // protect the view normally rendered and 
redirect to error response view
                            returnString = returnString.replace(":_protect_:", 
"");
                            if (returnString.length() > 0) {
                                request.setAttribute("_ERROR_MESSAGE_", 
returnString);
                            }
                            eventReturn = null;
                            // check to see if there is a "protect" response, 
if so it's ok else show the default_error_response_view
                            if 
(!requestMap.requestResponseMap.containsKey("protect")) {
                                String protectView = 
controllerConfig.getProtectView();
                                if (protectView != null) {
                                    overrideViewUri = protectView;
                                } else {
                                    overrideViewUri = 
UtilProperties.getPropertyValue("security.properties", 
"default.error.response.view");
                                    overrideViewUri = 
overrideViewUri.replace("view:", "");
                                    if ("none:".equals(overrideViewUri)) {
                                        interruptRequest = true;
                                    }
                                }
                            }
                        }
                    } else if (returnString == null) {
                        interruptRequest = true;
                    }
                } catch (EventHandlerException e) {
                    Debug.logError(e, module);
                }
            }
{code}

This leads to a flood of Exception stacktraces in the OFBiz logs.

In my opinion, the checkTrackingCodeUrlParam method should return "success" if 
the tracking code is not found. It's not an error.

What do others think?



> TrackingCodeEvents#checkTrackingCodeUrlParam returns "error" if tracking code 
> is not available
> ----------------------------------------------------------------------------------------------
>
>                 Key: OFBIZ-10030
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-10030
>             Project: OFBiz
>          Issue Type: Bug
>          Components: marketing
>    Affects Versions: Trunk
>            Reporter: Michael Brohl
>            Assignee: Michael Brohl
>            Priority: Minor
>
> In TrackingCodeEvents#checkTrackingCodeUrlParam, a provided tracking code is 
> checked against the database. If not found, it returns "error".
> {code:java}
>             if (trackingCode == null) {
>                 Debug.logError("TrackingCode not found for trackingCodeId [" 
> + trackingCodeId + "], ignoring this trackingCodeId.", module);
>                 //this return value will be ignored, but we'll designate this 
> as an error anyway
>                 return "error";
>             }
> {code}
> The notice "return value is ignored" is not true because in the event 
> handler, a non-success return value is handled to throw an excpetion:
> {code:java}
>             // Invoke the pre-processor (but NOT in a chain)
>             for (ConfigXMLReader.Event event: 
> controllerConfig.getPreprocessorEventList().values()) {
>                 try {
>                     String returnString = this.runEvent(request, response, 
> event, null, "preprocessor");
>                     if (returnString != null && 
> !returnString.equalsIgnoreCase("success")) {
>                         if (!returnString.contains(":_protect_:")) {
>                             throw new EventHandlerException("Pre-Processor 
> event [" + event.invoke + "] did not return 'success'.");
>                         } else { // protect the view normally rendered and 
> redirect to error response view
>                             returnString = 
> returnString.replace(":_protect_:", "");
>                             if (returnString.length() > 0) {
>                                 request.setAttribute("_ERROR_MESSAGE_", 
> returnString);
>                             }
>                             eventReturn = null;
>                             // check to see if there is a "protect" response, 
> if so it's ok else show the default_error_response_view
>                             if 
> (!requestMap.requestResponseMap.containsKey("protect")) {
>                                 String protectView = 
> controllerConfig.getProtectView();
>                                 if (protectView != null) {
>                                     overrideViewUri = protectView;
>                                 } else {
>                                     overrideViewUri = 
> UtilProperties.getPropertyValue("security.properties", 
> "default.error.response.view");
>                                     overrideViewUri = 
> overrideViewUri.replace("view:", "");
>                                     if ("none:".equals(overrideViewUri)) {
>                                         interruptRequest = true;
>                                     }
>                                 }
>                             }
>                         }
>                     } else if (returnString == null) {
>                         interruptRequest = true;
>                     }
>                 } catch (EventHandlerException e) {
>                     Debug.logError(e, module);
>                 }
>             }
> {code}
> This leads to a flood of Exception stacktraces in the OFBiz logs.
> In my opinion, the checkTrackingCodeUrlParam method should return "success" 
> if the tracking code is not found and should log to loglevel "info". It's not 
> an error.
> What do others think?



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to