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 Watcher crash if payload is less than 128 bytes #258

Closed
johnmurphy01 opened this issue May 14, 2019 · 9 comments
Closed

Subscription Watcher crash if payload is less than 128 bytes #258

johnmurphy01 opened this issue May 14, 2019 · 9 comments
Labels
bug Something isn't working

Comments

@johnmurphy01
Copy link
Contributor

Describe the bug
If a subscription's payload contains less than 128 bytes of data, the sdk will crash here: https://github.com/awslabs/aws-mobile-appsync-sdk-ios/blob/master/AWSAppSyncClient/AWSAppSyncSubscriptionWatcher.swift#L258

To Reproduce
Steps to reproduce the behavior:

  1. Perform the steps necessary to send under 128 bytes of data in a subscription's payload
  2. See error

Expected behavior
The sdk should not throw an exception on the verbose log line

Per the documentation on prefix: https://developer.apple.com/documentation/foundation/data/1780322-prefix

Environment(please complete the following information):

  • AppSync SDK Version: 2.13.0
  • Dependency Manager: Cocoapods
  • Swift Version : any

Device Information (please complete the following information):

  • Device: any
  • iOS Version: 11
  • Specific to simulators: No
@johnmurphy01
Copy link
Contributor Author

@palpatim I will gladly make a PR to fix this if you can let me know if you'd rather:
omit that log line altogether
or
adjust the prefix so that it won't ever go out of bounds

@johnmurphy01
Copy link
Contributor Author

BTW, this is the issue I was experiencing with #216, I just now caught it after troubleshooting today

@Eno8mayank
Copy link

Is there a fix for this? I am facing the same issue. The SDK crashes if payload is less than 128 bytes.

@johnmurphy01
Copy link
Contributor Author

I have a PR open but hasn't been approved or merged yet. I had to apply a hotfix by patching the Pod as a temporary fix

@Eno8mayank
Copy link

I have a PR open but hasn't been approved or merged yet. I had to apply a hotfix by patching the Pod as a temporary fix

Yes, thanks. I saw your patch and that worked for me as well.

@royjit
Copy link
Contributor

royjit commented Aug 6, 2019

Thank you @johnmurphy01 I have approved and merged the PR. Please reopen if you are seeing any other issue.

@royjit royjit closed this as completed Aug 6, 2019
@royjit
Copy link
Contributor

royjit commented Aug 7, 2019

@johnmurphy01 Do you think the server side fix that you mentioned here #216 was needed to make this work?

@johnmurphy01
Copy link
Contributor Author

No. This was an attempt to fix the problem before we knew what was going on. We have since reverted those server side changes and the change that you merged yesterday is still working

@royjit
Copy link
Contributor

royjit commented Aug 8, 2019

@johnmurphy01 thank you for confirming

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

4 participants