[jira] [Updated] (OFBIZ-5409) JSON Response does not set http status on error

["Jacques Le Roux (JIRA)" <ji...@apache.org>]

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

Jacques Le Roux updated OFBIZ-5409:
-----------------------------------

    Attachment: before-after.diff
                OFBIZ-5409 - Remove internal attributes for security reason.patch

Gareth,

{quote}
The list of attributes was bad practise on my part, I noticed ofbiz returning these attributes whilst testing returning 500 status code and was testing ignoring them and should have implemented it properly (and separately).
{quote}

It seems you catched all(most?) of them, see for instance the attached before/after diff for getCountryList request. If ever someone find a case where an internal attribute is still returned then we will add it in the list, good job!

{quote}
On reflection, I think you're right about the status code, always return 200 except for exceptions and client errors aswell. I was trying to remove any duplicate error handling from Javascript side by having a single error function, is this something that could be integrated into ofbiz?
{quote}

Yes this could be added to the getServerError() function you can find in 3 js files in OFBiz. BTW while at it maybe we could refactor if it's really duplicated (sorry lack of time to check) in a sole function in a new util.js file which could contains also some other internal duplicates we have.
{code}
// Check server side error
function getServerError(data) {
...
}
{code}

{quote}
Jacques, I ignored all attributes that were returned on a specific request, most likely those ones are for all requests and will be incomplete. My opinion, it should only return the result from a web service, all other attributes should be ignored as there is no guarantee what can be returned.
{quote}
Sure thing, though it would need a much complicated mechanism to keep only _ERROR_MESSAGE_ and the initially called service attributes. So I will commit your (slightly modified) patch (with ignoreAttrs as static final) but  I'm still undecided on the HttpServletResponse.SC_INTERNAL_SERVER_ERROR part. It seems it has no bad side effects, but like Adrian, I'm not sure it's helpful, maybe we should rather improve getServerError()...

I will also change the comment "uses all compatible request attributes" there (3 occurences) for 

{code}
    <!-- Common json reponse events, chain these after events to send json reponses -->
    <!-- Standard json response, uses all compatible request attributes -->
    <request-map uri="json">
        <security direct-request="false"/>
        <event type="java" path="org.ofbiz.common.CommonEvents" invoke="jsonResponseFromRequestAttributes"/>
        <response name="success" type="none"/>
    </request-map>
{code}

For now I attach my proposition as OFBIZ-5409 - Remove internal attributes for security reason.patch

> JSON Response does not set http status on error
> -----------------------------------------------
>
>                 Key: OFBIZ-5409
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-5409
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: ALL APPLICATIONS
>    Affects Versions: SVN trunk
>            Reporter: Gareth Carter
>            Priority: Trivial
>         Attachments: CommonEvents.patch, OFBIZ-5409 - Remove internal attributes for security reason.patch, before-after.diff
>
>
> When a json response is sent and there was an error in the service called, it does not set the http status. Currently status code is always 200 but it might be more appropriate to send an error code such as 500.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)