You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by "Bob Morley (JIRA)" <ji...@apache.org> on 2010/04/14 15:50:51 UTC

[jira] Created: (OFBIZ-3699) ServiceDispatcher.checkAuth modifies the context if the invocation service has a permissionServiceName

ServiceDispatcher.checkAuth modifies the context if the invocation service has a permissionServiceName
------------------------------------------------------------------------------------------------------

                 Key: OFBIZ-3699
                 URL: https://issues.apache.org/jira/browse/OFBIZ-3699
             Project: OFBiz
          Issue Type: Bug
          Components: framework
    Affects Versions: SVN trunk
            Reporter: Bob Morley
             Fix For: SVN trunk


Created as a result of thread: http://n4.nabble.com/Magically-converted-types-from-simpleTypeConvert-td1838891.html

The follow code in the ServiceDispatcher ...

        if (UtilValidate.isNotEmpty(origService.permissionServiceName)) {
            ...
            if (hasPermission.booleanValue()) {
                context.putAll(permResp);
                context = origService.makeValid(context, ModelService.IN_PARAM); 

... causes the incoming context to be modified both by adding values from the results of the permission service but also by converting any datatypes to match those in the service definition.  This hides any invalid service invocations (from a data type pov) and if the permisionServiceName is removed, the code would start failing with the incorrect data types.

Suggest is to change this to something like ...

Map<String, Object> permRespContext = ServiceUtil.setServiceFields(dctx, serviceName, permResp);
context.putAll(permRespContext); 

The concern is that by doing this there may be some services that were relying on the data type conversion (because they were invalid requests) which would start to fail.  Appropriate impact analysis of services that define "permissionServiceName" and appropriate resolutions need to be included with this change.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (OFBIZ-3699) ServiceDispatcher.checkAuth modifies the context if the invocation service has a permissionServiceName

Posted by "Scott Gray (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OFBIZ-3699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12865635#action_12865635 ] 

Scott Gray commented on OFBIZ-3699:
-----------------------------------

I had a go at fixing this over the weekend but the unit tests fail because we already have code that is passing invalid parameters to service calls.  What I would like to do is fix the bug before any more damage is done and just disable the offending unit tests until they can be corrected.
Committing the fix will most likely expose new bugs in the regular code but there isn't really much we can do to avoid that IMO.

> ServiceDispatcher.checkAuth modifies the context if the invocation service has a permissionServiceName
> ------------------------------------------------------------------------------------------------------
>
>                 Key: OFBIZ-3699
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-3699
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>            Reporter: Bob Morley
>             Fix For: SVN trunk
>
>
> Created as a result of thread: http://n4.nabble.com/Magically-converted-types-from-simpleTypeConvert-td1838891.html
> The follow code in the ServiceDispatcher ...
>         if (UtilValidate.isNotEmpty(origService.permissionServiceName)) {
>             ...
>             if (hasPermission.booleanValue()) {
>                 context.putAll(permResp);
>                 context = origService.makeValid(context, ModelService.IN_PARAM); 
> ... causes the incoming context to be modified both by adding values from the results of the permission service but also by converting any datatypes to match those in the service definition.  This hides any invalid service invocations (from a data type pov) and if the permisionServiceName is removed, the code would start failing with the incorrect data types.
> Suggest is to change this to something like ...
> Map<String, Object> permRespContext = ServiceUtil.setServiceFields(dctx, serviceName, permResp);
> context.putAll(permRespContext); 
> The concern is that by doing this there may be some services that were relying on the data type conversion (because they were invalid requests) which would start to fail.  Appropriate impact analysis of services that define "permissionServiceName" and appropriate resolutions need to be included with this change.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OFBIZ-3699) ServiceDispatcher.checkAuth modifies the context if the invocation service has a permissionServiceName

Posted by "Bob Morley (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OFBIZ-3699?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12865764#action_12865764 ] 

Bob Morley commented on OFBIZ-3699:
-----------------------------------

Yes I think that was my original concern.  What you probably can not do (very easily) is find all of the actual production code that is passing invalid parameters.  I agree we should get this in before any more damage is done, but I think we need to understand that this will spawn a number of "invalid service invocation" tickets.

I originally came across this with creation of a ProductionRun as indicated in the referenced thread.  That is one spot where we will start to break with this fix.  Should we create a single parent ticket where all of the instances of failure can be recorded and we can resolve them piecemeal?

> ServiceDispatcher.checkAuth modifies the context if the invocation service has a permissionServiceName
> ------------------------------------------------------------------------------------------------------
>
>                 Key: OFBIZ-3699
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-3699
>             Project: OFBiz
>          Issue Type: Bug
>          Components: framework
>    Affects Versions: SVN trunk
>            Reporter: Bob Morley
>             Fix For: SVN trunk
>
>
> Created as a result of thread: http://n4.nabble.com/Magically-converted-types-from-simpleTypeConvert-td1838891.html
> The follow code in the ServiceDispatcher ...
>         if (UtilValidate.isNotEmpty(origService.permissionServiceName)) {
>             ...
>             if (hasPermission.booleanValue()) {
>                 context.putAll(permResp);
>                 context = origService.makeValid(context, ModelService.IN_PARAM); 
> ... causes the incoming context to be modified both by adding values from the results of the permission service but also by converting any datatypes to match those in the service definition.  This hides any invalid service invocations (from a data type pov) and if the permisionServiceName is removed, the code would start failing with the incorrect data types.
> Suggest is to change this to something like ...
> Map<String, Object> permRespContext = ServiceUtil.setServiceFields(dctx, serviceName, permResp);
> context.putAll(permRespContext); 
> The concern is that by doing this there may be some services that were relying on the data type conversion (because they were invalid requests) which would start to fail.  Appropriate impact analysis of services that define "permissionServiceName" and appropriate resolutions need to be included with this change.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.