Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
gmulhearn opened this issue Jul 1, 2021 · 0 comments
Labels
bug Something isn't working

Comments

@gmulhearn
Copy link

gmulhearn commented Jul 1, 2021

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):

  • AppSync SDK Version: 3.1.4

Device Information (please complete the following information):

  • Device: Any
  • Android API Version: from 19 to 30

Additional context
Add any other context about the problem here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants