wso2 / charon

120 stars 162 forks source link

`java.lang.ClassCastException` when PATCH `value` is null #369

Open kenston opened 2 years ago

kenston commented 2 years ago

Description: When sending PATCH /Users/{{userId}} call with null value:

{
    "schemas": [
        "urn:ietf:params:scim:api:messages:2.0:PatchOp"
    ],
    "Operations": [
        {
            "op": "add",
            "path": "preferredLanguage",
            "value": null
        }
    ]
}

Exception happens:

Caused by: java.lang.ClassCastException: org.json.JSONObject$Null cannot be cast to java.lang.String
    at org.wso2.charon3.core.attributes.SimpleAttribute.getStringValue(SimpleAttribute.java:81)
    at org.wso2.charon3.core.utils.LambdaExceptionUtils.lambda$rethrowFunction$2(LambdaExceptionUtils.java:84)
    at org.wso2.charon3.core.objects.User.lambda$getPreferredLanguage$5(User.java:175)
    at java.util.Optional.map(Optional.java:215)
    at org.wso2.charon3.core.objects.User.getPreferredLanguage(User.java:174)
    ...

Suggested Labels: Type/Bug

Affected Product Version:

3.4.23 but likely in current version

Steps to reproduce:

Send a PATCH operation whose value is null.

In https://github.com/wso2/charon/blob/5d5009c1eff2f501244420404a7b4983f3fc8e6b/modules/charon-core/src/main/java/org/wso2/charon3/core/attributes/SimpleAttribute.java#L77-L85, this.value was org.json.JSONObject$Null so it should return null to avoid doing the casting.

Issue may not be limited to just preferredLanguage, but let's say it's:

        {
            "op": "add",
            "path": "active"
            "value": ""
        },

It will also hit similar issue:

Caused by: java.lang.ClassCastException: java.lang.String cannot be cast to java.lang.Boolean
    at org.wso2.charon3.core.attributes.SimpleAttribute.getBooleanValue(SimpleAttribute.java:120)
    at org.wso2.charon3.core.utils.LambdaExceptionUtils.lambda$rethrowFunction$2(LambdaExceptionUtils.java:84)

And those ClassCastException happen only for PATCH. With POST (User create) or PUT (Replace), having it as null for preferredLanguage or "" for active did not hit any issue.

Related Issues: https://github.com/wso2/charon/issues/314

cb-manideep commented 1 month ago

I'm using WSO2 charon version 4.0.18 and I get the same issue, I believe this part of the code is assuming the following two things: https://github.com/wso2/charon/blob/73257cc788b3224559fb26f9ebc8853e8b5019c8/modules/charon-core/src/main/java/org/wso2/charon3/core/utils/PatchOperationUtil.java#L1856

  1. The value in the PATCH operation cannot be null
  2. The value in the PATCH operation is always a string

This would also cause a java.lang.ClassCastException when the PATCH request body which has path value replacement instead of non-path value replacement

{
  "Operations": [
    {
      "op": "replace",
      "path": "active",
      "value": false
    }
  ],
  "schemas": [
    "urn:ietf:params:scim:api:messages:2.0:PatchOp"
  ]
}

The exception:

Caused by: java.lang.ClassCastException: java.lang.String cannot be cast to java.lang.Boolean
    at org.wso2.charon3.core.attributes.SimpleAttribute.getBooleanValue(SimpleAttribute.java:120) ~[org.wso2.charon3.core-4.0.18.jar:?]
    at org.wso2.charon3.core.utils.LambdaExceptionUtils.lambda$rethrowFunction$2(LambdaExceptionUtils.java:84) ~[org.wso2.charon3.core-4.0.18.jar:?]
    at org.wso2.charon3.core.objects.User.lambda$getActive$8(User.java:254) ~[org.wso2.charon3.core-4.0.18.jar:?]
    at java.util.Optional.map(Optional.java:215) ~[?:1.8.0_362]
    at org.wso2.charon3.core.objects.User.getActive(User.java:254) ~[org.wso2.charon3.core-4.0.18.jar:?]

I think instead of always casting it to a string, we should check the datatype and convert it and set the value to the Object or set the value directly

cb-manideep commented 1 month ago

I've solved this issue temporarily by overriding the User#getActive and type casting based on the attribute

public class CharonUser extends User {
    public CharonUser(User user) {
        for (Attribute attribute : user.getAttributeList().values()) {
            setAttribute(attribute);
        }
        for (String schema : user.getSchemaList()) {
            setSchema(schema);
        }
    }

    @Override
    public boolean getActive() {
        SCIMAttributeSchema attributeDefinition = SCIMSchemaDefinitions.SCIMUserSchemaDefinition.ACTIVE;
        return this.getSimpleAttribute(attributeDefinition).map(simpleAttribute -> rethrowSupplier(
                        () -> {
                            Object value = simpleAttribute.getValue();
                            if (value instanceof Boolean) {
                                return (Boolean) value;
                            } else if (value instanceof String) {
                                return Boolean.parseBoolean((String) value);
                            } else {
                                throw new CharonException("Datatype doesn't match the datatype of the attribute value");
                            }
                        }
                ).get()
        ).orElse(false);
    }
}

Note: This is not a permanent solution and will need to verify if there are any changes in User and it's parent classes to see if the implementation has been changed if you are updating the new version of this library. I'm using 4.0.18 and it works with this version.