awslabs / aws-mobile-appsync-sdk-android

Android SDK for AWS AppSync.
https://docs.amplify.aws/sdk/api/graphql/q/platform/android/
Apache License 2.0
105 stars 58 forks source link

Subscription ENUM input variables are nulled before being sent out via websocket and startSubscription #352

Open gmulhearn opened 3 years ago

gmulhearn commented 3 years ago

Describe the bug The startSubscription method in WebSocketConnectionManager attempts to convert a Subscription's variables into JSON by just calling JSONObject(map) over the subscription's valueMap() (see line 123 of WebSocketConnectionManager).

This is dangerous, because the JSONObject(map) constructor, in android API 19 to 30, cannot handle maps that have values which are enums, it nulls them out (specifically, see the wrap(Object o) method of JSONObject in these API versions).

This means that when a Subscription has a non-nullable input variable that is an apollo generated ENUM, WebSocketConnectionManager will null it before sending it to the service, which ofcourse will cause the service to throw an error.

WebSocketConnectionManager should use the generated Variables.marshaller().marshal(..) method to properly JSON-ify the subscription's variables. This can be seen in PR #351 .

To Reproduce Steps to reproduce the behavior:

  1. create a graphql subscription with a non-nullable ENUM as input.
  2. attempt to execute a subscription execution on that subscription
  3. observe that line 123 of WebSocketConnectionManager nulls the enum's value in the JSON Object.
  4. observe error message recieved from backend on that subscription.

To Reproduce via Unit Test to quickly observe the nulling effect, run the following unit test:

@RunWith(RobolectricTestRunner.class)
@Config(sdk = 28)
public class WebSocketConnectionManagerUnitTest {

    Subscription<?, ?, ?> mockSubscription;
    Operation.Variables mockVariables;

    Map<String, Object> mapOfPrimitivesAndEnums = new HashMap<>();

    @Test
    public void testSubscriptionValueMapWithEnumIsInvalidJSON() {
        mockSubscription = Mockito.mock(Subscription.class);
        mockVariables = Mockito.mock(Operation.Variables.class);

        mapOfPrimitivesAndEnums.put("str", "strVal");
        mapOfPrimitivesAndEnums.put("enum", TestEnum.ENUM_VALUE);

        Mockito.when(mockSubscription.variables()).thenReturn(mockVariables);
        Mockito.when(mockVariables.valueMap()).thenReturn(Collections.unmodifiableMap(mapOfPrimitivesAndEnums));

        JSONObject jsonObject = new JSONObject(mockSubscription.variables().valueMap());
        assertNull(jsonObject.opt("enum"));  // check that enum has been nulled out.
    }
}

enum TestEnum {
    ENUM_VALUE
}

Expected behavior Enums sent via WebSocketConnectionManager should NOT be nulled before sent out. I imagine this is probably a bug for any non primitive object too (my PR should fix), i just happened to have observed it for enums.

Screenshots If applicable, add screenshots to help explain your problem.

Environment(please complete the following information):

Device Information (please complete the following information):

Additional context Add any other context about the problem here.