Skip to content

Commit 1023b01

Browse files
committed
Fix crash from $ in external user ID
* If a user has a dollar sign ($) in the external user ID, and our code is trying to escape the forward slashes via a string replacement, this will cause a crash as `$` has a non-literal meaning when used in the replacement string. The solution is to call `quoteReplacement` to escape any $ or \ signs. See https://docs.oracle.com/javase/8/docs/api/java/lang/String.html#replaceAll-java.lang.String-java.lang.String- * Also fix the pattern matching for external user ID. Previously in a JSONObject like {"app_id": "abc", "external_user_id": "user1", "timezone": "Europe/London"}, the regex would match `def", "timezone": "Europe/London`, grabbing the forward slash in ANY values that come after the external_user_id. Fix this to match the external user ID value only.
1 parent 5f40a60 commit 1023b01

File tree

4 files changed

+99
-14
lines changed

4 files changed

+99
-14
lines changed

OneSignalSDK/onesignal/src/main/java/com/onesignal/JSONUtils.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@
1313
import java.util.List;
1414
import java.util.Map;
1515
import java.util.Set;
16+
import java.util.regex.Matcher;
17+
import java.util.regex.Pattern;
18+
19+
import static com.onesignal.UserStateSynchronizer.EXTERNAL_USER_ID;
1620

1721

1822
class JSONUtils {
@@ -130,6 +134,32 @@ static String toStringNE(JSONArray jsonArray) {
130134
return strArray + "]";
131135
}
132136

137+
/**
138+
* Returns the JSONObject as a String with the external user ID unescaped.
139+
* Needed b/c the default JSONObject.toString() escapes (/) with (\/), which customers may not want.
140+
*/
141+
static String toUnescapedEUIDString(JSONObject json) {
142+
String strJsonBody = json.toString();
143+
144+
if (json.has(EXTERNAL_USER_ID)) {
145+
// find the value of the external user ID
146+
Pattern eidPattern = Pattern.compile("(?<=\"external_user_id\":\").*?(?=\")");
147+
Matcher eidMatcher = eidPattern.matcher(strJsonBody);
148+
149+
if (eidMatcher.find()) {
150+
String matched = eidMatcher.group(0);
151+
if (matched != null) {
152+
String unescapedEID = matched.replace("\\/", "/");
153+
// backslashes (\) and dollar signs ($) in the replacement string will be treated literally
154+
unescapedEID = eidMatcher.quoteReplacement(unescapedEID);
155+
strJsonBody = eidMatcher.replaceAll(unescapedEID);
156+
}
157+
}
158+
}
159+
160+
return strJsonBody;
161+
}
162+
133163
static JSONObject getJSONObjectWithoutBlankValues(ImmutableJSONObject jsonObject, String getKey) {
134164
if (!jsonObject.has(getKey))
135165
return null;

OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignalRestClient.java

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@
4040
import java.net.HttpURLConnection;
4141
import java.net.URL;
4242
import java.util.Scanner;
43-
import java.util.regex.Matcher;
44-
import java.util.regex.Pattern;
4543

4644
import javax.net.ssl.HttpsURLConnection;
4745

@@ -168,18 +166,7 @@ private static Thread startHTTPConnection(String url, String method, JSONObject
168166
}
169167

170168
if (jsonBody != null) {
171-
String strJsonBody = jsonBody.toString();
172-
173-
Pattern eidPattern = Pattern.compile("(?<=\"external_user_id\":\").*\\\\/.*?(?=\",|\"\\})");
174-
Matcher eidMatcher = eidPattern.matcher(strJsonBody);
175-
176-
if (eidMatcher.find()) {
177-
String matched = eidMatcher.group(0);
178-
if (matched != null) {
179-
String unescapedEID = matched.replace("\\/", "/");
180-
strJsonBody = eidMatcher.replaceAll(unescapedEID);
181-
}
182-
}
169+
String strJsonBody = JSONUtils.toUnescapedEUIDString(jsonBody);
183170

184171
OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "OneSignalRestClient: " + method + " SEND JSON: " + strJsonBody);
185172

OneSignalSDK/unittest/src/test/java/com/onesignal/OneSignalPackagePrivateHelper.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ public static JSONObject bundleAsJSONObject(Bundle bundle) {
129129
return NotificationBundleProcessor.bundleAsJSONObject(bundle);
130130
}
131131

132+
public static String toUnescapedEUIDString(JSONObject json) {
133+
return JSONUtils.toUnescapedEUIDString(json);
134+
}
135+
132136
public static void OneSignal_handleNotificationOpen(Activity context, final JSONArray data, final String notificationId) {
133137
OneSignal.handleNotificationOpen(context, data, notificationId);
134138
}

OneSignalSDK/unittest/src/test/java/com/test/onesignal/MainOneSignalClassRunner.java

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@
141141
import static com.onesignal.OneSignalPackagePrivateHelper.OneSignal_setTrackerFactory;
142142
import static com.onesignal.OneSignalPackagePrivateHelper.OneSignal_taskQueueWaitingForInit;
143143
import static com.onesignal.OneSignalPackagePrivateHelper.OSObservable;
144+
import static com.onesignal.OneSignalPackagePrivateHelper.toUnescapedEUIDString;
144145
import static com.onesignal.ShadowOneSignalRestClient.EMAIL_USER_ID;
145146
import static com.onesignal.ShadowOneSignalRestClient.PUSH_USER_ID;
146147
import static com.onesignal.ShadowOneSignalRestClient.REST_METHOD;
@@ -3378,6 +3379,69 @@ public void testOSNotificationOpenResultToJSONObject() throws Exception {
33783379
assertEquals("collapseId1", firstGroupedNotification.optString("collapseId"));
33793380
}
33803381

3382+
// ####### Unit test JSONUtils methods
3383+
@Test
3384+
public void test_JSONUtils_toUnescapedEUIDString() throws Exception {
3385+
// 1. Test when EUID is first in the json, and has ($) and (/), and ($) elsewhere
3386+
3387+
// Set up the JSONObject to test with
3388+
String jsonStringWithDollarAndSlash = "{" +
3389+
"\"external_user_id\":\"$1$/abc/de$f/\"," +
3390+
"\"app_id\":\"b4f7f966-d8cc-11e4-bed1-df8f05be55ba\"," +
3391+
"\"timezone\":\"$Europe/London\"" +
3392+
"}";
3393+
JSONObject jsonWithDollarAndSlash = new JSONObject(jsonStringWithDollarAndSlash);
3394+
3395+
// The expected string which escapes the "timezone" slash (/) only
3396+
String expected_jsonStringWithDollarAndSlash = "{" +
3397+
"\"external_user_id\":\"$1$/abc/de$f/\"," +
3398+
"\"app_id\":\"b4f7f966-d8cc-11e4-bed1-df8f05be55ba\"," +
3399+
"\"timezone\":\"$Europe\\/London\"" +
3400+
"}";
3401+
3402+
// The actual string result from calling JSONUtils.toUnescapedEUIDString()
3403+
String actual_jsonStringWithDollarAndSlash = toUnescapedEUIDString(jsonWithDollarAndSlash);
3404+
3405+
// These two strings should be equal
3406+
assertEquals(expected_jsonStringWithDollarAndSlash, actual_jsonStringWithDollarAndSlash);
3407+
3408+
// 2. Test when EUID is first in the json, and has no dollar nor slash
3409+
3410+
String jsonStringWithEUID = "{" +
3411+
"\"external_user_id\":\"123abc\"," +
3412+
"\"app_id\":\"b4f7f966-d8cc-11e4-bed1-df8f05be55ba\"," +
3413+
"\"timezone\":\"$Europe/London\"" +
3414+
"}";
3415+
JSONObject jsonWithEUID = new JSONObject(jsonStringWithEUID);
3416+
3417+
String expected_jsonStringWithEUID = "{" +
3418+
"\"external_user_id\":\"123abc\"," +
3419+
"\"app_id\":\"b4f7f966-d8cc-11e4-bed1-df8f05be55ba\"," +
3420+
"\"timezone\":\"$Europe\\/London\"" +
3421+
"}";
3422+
3423+
String actual_jsonStringWithEUID = toUnescapedEUIDString(jsonWithEUID);
3424+
3425+
assertEquals(expected_jsonStringWithEUID, actual_jsonStringWithEUID);
3426+
3427+
// 3. Test when there is no EUID is in the json
3428+
3429+
String jsonStringWithoutEUID = "{" +
3430+
"\"app_id\":\"b4f7f966-d8cc-11e4-bed1-df8f05be55ba\"," +
3431+
"\"timezone\":\"Europe/London\"" +
3432+
"}";
3433+
JSONObject jsonWithoutEUID = new JSONObject(jsonStringWithoutEUID);
3434+
3435+
String expected_jsonStringWithoutEUID = "{" +
3436+
"\"app_id\":\"b4f7f966-d8cc-11e4-bed1-df8f05be55ba\"," +
3437+
"\"timezone\":\"Europe\\/London\"" +
3438+
"}";
3439+
3440+
String actual_jsonStringWithoutEUID = toUnescapedEUIDString(jsonWithoutEUID);
3441+
3442+
assertEquals(expected_jsonStringWithoutEUID, actual_jsonStringWithoutEUID);
3443+
}
3444+
33813445
@Test
33823446
public void testNotificationOpenedProcessorHandlesEmptyIntent() {
33833447
NotificationOpenedProcessor_processFromContext(blankActivity, new Intent());

0 commit comments

Comments
 (0)