Fix crash in OneSignalAttachmentHandler trimURLSpacing #1411
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* The method `trimURLSpacing` is defined in both OneSignalCoreHelper and OneSignalAttachmentHandler * Let's just use it in one place, the OneSignalCoreHelper, as this method is also used elsewhere.
* This is not testable in Swift due to type safety, making it hard to pass in NSNull * But it is very possible for this method that expects NSString to receive NSNull due to processing a payload * Therefore, a test file in OneSignalCoreTests is created in Objective-C * As tests expand, these can be moved to more specific files
nan-li
changed the title
1 task
nan-li
changed the title
nan-li
requested review from
jkasten2,
emawby,
jinliu9508,
shepherd-l and
jennantilla
jkasten2
approved these changes
Apr 22, 2024
jennantilla
approved these changes
Apr 22, 2024
This was referenced Apr 30, 2024
Merged
This was referenced May 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
One Line Summary
Fix crash in
OneSignalAttachmentHandlertrimURLSpacingby checking forNSNullin addition tonilchecking that is already existent.Details
The attachment handler can have values in a dictionary be
NSNullsince is used to represent nil objects in collections, and we typically seeNSNullwhen we convert JSON to Objective-C objects and the JSON contained null values.Motivation
Scope
Checking for nullness
Testing
Unit testing
Added one unit test
Manual testing
Reproduced the crash by passing in
NSNullmanually.Tested on iPhone 13 ios 17.4 by attaching an image and seeing method triggered.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is