diff --git a/CHANGELOG.md b/CHANGELOG.md index f1154a50b..af9a62fc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ ## XX.XX.XX +* Improved link handling for content and feedback widgets, so links that carry their own query parameters, such as deep links, are parsed correctly. * Added a content configuration option to provide a handler for links opened from the content web view, so the app can route its own deep links instead of the SDK opening the system browser, set via `setContentUrlHandler(ContentUrlHandler)`. ## 26.1.4 diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java index 29c83f248..b27c42279 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java @@ -22,6 +22,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import static org.mockito.Mockito.mock; /** * Instrumented tests for ContentOverlayView. @@ -128,6 +129,13 @@ private WindowManager.LayoutParams invokeCreateWindowParams( return (WindowManager.LayoutParams) method.invoke(overlay, activity, config); } + @SuppressWarnings("unchecked") + private Map invokeSplitQuery(String url) throws Exception { + Method method = ContentOverlayView.class.getDeclaredMethod("splitQuery", String.class); + method.setAccessible(true); + return (Map) method.invoke(overlay, url); + } + /** * Launches the test activity, runs the given action on the main thread, * and stores the scenario for cleanup. @@ -212,6 +220,37 @@ public void contentUrlAction_multipleEvents_returnsTrue() { }); } + /** + * Malformed "event" JSON must not crash. When the payload fails to validate, splitQuery stores + * it as a raw String (not a JSONArray) and still routes action=event to eventAction. eventAction + * must guard the cast instead of letting a ClassCastException propagate out of + * shouldOverrideUrlLoading. The URL is still a consumed countly action URL (returns true) and + * simply records nothing. Regression test for the unguarded (JSONArray) cast. + */ + @Test + public void contentUrlAction_malformedEventJson_doesNotThrow() { + withActivity(activity -> { + overlay = createOverlay(activity); + // Truncated JSON array: new JSONArray(...) throws, so "event" is not stored as a JSONArray. + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=event&event=[{\"key\":\"oops\""; + Assert.assertTrue(overlay.contentUrlAction(url, overlay.webView)); + }); + } + + /** + * Same guard with reversed param order: a valid action=event verb paired with a malformed event + * payload that appears before it. The event value lands in splitQuery's fallback prefix as a + * String, so eventAction must not cast-crash. + */ + @Test + public void contentUrlAction_malformedEventJson_reversedOrder_doesNotThrow() { + withActivity(activity -> { + overlay = createOverlay(activity); + String url = Utils.COMM_URL + "/?cly_x_action_event=1&event=[oops&action=event"; + Assert.assertTrue(overlay.contentUrlAction(url, overlay.webView)); + }); + } + /** * resize_me action parses JSON and updates portrait/landscape configs. */ @@ -405,6 +444,382 @@ public void widgetUrlAction_nullWebView_returnsFalse() { }); } + // ===================== link parsing — query param preservation ===================== + + /** + * A link with a single query param is preserved intact (the raw, unencoded form the server + * sends). Regression guard: previously the naive '&' split truncated it at the first '&'. + */ + @Test + public void splitQuery_linkWithSingleQueryParam_preserved() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "https://example.com/path?foo=bar"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + link; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + Assert.assertEquals("link", q.get("action")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * A link whose own query string has MULTIPLE '&'-separated params keeps all of them. This is + * the core fix: everything after "link=" is captured verbatim instead of being split on '&'. + */ + @Test + public void splitQuery_linkWithMultipleQueryParams_preserved() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "https://example.com/path?foo=bar&baz=qux&n=42"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + link; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + // The link's own params must NOT leak in as separate top-level entries. + Assert.assertFalse(q.containsKey("baz")); + Assert.assertFalse(q.containsKey("n")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * A deeplink with query params (custom scheme) is preserved intact. + */ + @Test + public void splitQuery_deeplinkWithQueryParams_preserved() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "myapp://open?screen=home&id=42&ref=push"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + link; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * "event" appearing AFTER the link is separated out: the reserved marker is validated as JSON, + * so it is peeled from the tail while the link (with its own query params) stays intact. + */ + @Test + public void splitQuery_eventAfterLink_separatedFromLink() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "https://x.com/p?a=b&c=d"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + link + + "&event=[{\"key\":\"e\",\"sg\":{\"x\":\"y\"}}]"; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + Assert.assertTrue(q.get("event") instanceof org.json.JSONArray); + Assert.assertEquals("e", ((org.json.JSONArray) q.get("event")).getJSONObject(0).getString("key")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * A "&event=" that appears inside the link but does NOT validate as JSON is treated as ordinary + * link text (not a real param) and stays part of the link. + */ + @Test + public void splitQuery_invalidReservedMarkerInLink_staysInLink() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "https://x.com/p?a=b&event=notjson"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + link; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + Assert.assertFalse(q.containsKey("event")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * A decoded "event" JSON whose segmentation value literally contains "&close=1" is parsed whole: + * the inner "&close=" fails close validation, so it is absorbed into the JSON value rather than + * being mistaken for a real close param. + */ + @Test + public void splitQuery_eventJsonContainingReservedText_parsedWhole() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=event" + + "&event=[{\"key\":\"k\",\"sg\":{\"u\":\"a&close=1\"}}]"; + Map q = invokeSplitQuery(url); + Assert.assertTrue(q.get("event") instanceof org.json.JSONArray); + Assert.assertEquals("a&close=1", + ((org.json.JSONArray) q.get("event")).getJSONObject(0).getJSONObject("sg").getString("u")); + Assert.assertFalse(q.containsKey("close")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * A link with no query params still works (no "link=" trailing content edge case). + */ + @Test + public void splitQuery_linkWithoutQueryParams_preserved() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "https://example.com/landing"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + link; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * Non-regression: an "event" JSON value (already decoded by the WebView client) is parsed into a + * JSONArray. The event does not co-occur with a link, so the whole query splits cleanly. + */ + @Test + public void splitQuery_eventValue_parsed() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=event" + + "&event=[{\"key\":\"test_key\",\"sg\":{\"color\":\"blue\"}}]"; + Map q = invokeSplitQuery(url); + Assert.assertTrue(q.get("event") instanceof org.json.JSONArray); + org.json.JSONArray arr = (org.json.JSONArray) q.get("event"); + Assert.assertEquals("test_key", arr.getJSONObject(0).getString("key")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * When a "close" flag is appended after a link (with the link carrying its own query params), + * the link is preserved intact AND the trailing close flag is peeled off and parsed separately + * rather than being swallowed into the link value. + */ + @Test + public void splitQuery_linkWithTrailingClose_separatesLinkAndClose() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "https://example.com/path?foo=bar&baz=qux"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + link + "&close=1"; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + Assert.assertEquals("1", q.get("close")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * "close" may also precede the link. In that case it stays in the head and is parsed normally, + * while the link (with its own query params) is still captured intact. + */ + @Test + public void splitQuery_closeBeforeLink_separatesLinkAndClose() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "https://example.com/path?foo=bar&baz=qux"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&close=1&link=" + link; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + Assert.assertEquals("1", q.get("close")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * Documented reserved-name limitation: because "close" is a reserved param, a link that itself + * ends with "&close=1" (a valid close value) has that segment consumed as the close flag. The + * link keeps everything up to it. Integrators are told these param names are reserved. + */ + @Test + public void splitQuery_linkEndingInReservedClose_consumedAsFlag() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=https://x.com?a=b&c=d&close=1&close=1"; + Map q = invokeSplitQuery(url); + Assert.assertEquals("https://x.com?a=b&c=d", q.get("link")); + Assert.assertEquals("1", q.get("close")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * A trailing "close=0" after a link is also peeled off (link kept open). + */ + @Test + public void splitQuery_linkWithTrailingCloseZero_separatesLinkAndClose() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "https://example.com/path?a=1&b=2"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + link + "&close=0"; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + Assert.assertEquals("0", q.get("close")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * End-to-end: a link action with query params AND a trailing close is recognized (returns true), + * dispatches the external intent, and closes the overlay. + */ + @Test + public void contentUrlAction_linkWithQueryParamsAndClose_closesOverlay() { + AtomicBoolean closeCalled = new AtomicBoolean(false); + withActivity(activity -> { + overlay = createOverlay(activity, null, () -> closeCalled.set(true)); + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link" + + "&link=https://example.com/path?foo=bar&baz=qux&close=1"; + Assert.assertTrue(overlay.contentUrlAction(url, overlay.webView)); + }); + Assert.assertTrue("overlay should close when close=1 trails the link", closeCalled.get()); + } + + /** + * End-to-end: a link action with query params (no close) is recognized and handled (returns + * true), and does not throw while dispatching the external intent. + */ + @Test + public void contentUrlAction_linkWithQueryParams_returnsTrue() { + withActivity(activity -> { + overlay = createOverlay(activity); + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link" + + "&link=https://example.com/path?foo=bar&baz=qux"; + Assert.assertTrue(overlay.contentUrlAction(url, overlay.webView)); + }); + } + + /** + * A link fragment ("#...") is preserved verbatim. + */ + @Test + public void splitQuery_linkWithFragment_preserved() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "https://example.com/path?a=b#section"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + link; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * A link containing more than one "?" is preserved verbatim (the "?" characters after the first + * are part of the link value, not new query separators). + */ + @Test + public void splitQuery_linkWithRepeatedQuestionMark_preserved() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "https://example.com/p?a=b?c=d"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + link; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * All three of link, event and close in one URL are each separated correctly, with the link + * (carrying its own query params) kept intact. + */ + @Test + public void splitQuery_linkEventAndClose_allSeparated() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "https://x.com/p?a=b&c=d"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + link + + "&event=[{\"key\":\"e\"}]&close=1"; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + Assert.assertEquals("1", q.get("close")); + Assert.assertTrue(q.get("event") instanceof org.json.JSONArray); + Assert.assertEquals("e", ((org.json.JSONArray) q.get("event")).getJSONObject(0).getString("key")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * A "close" whose value is not "0"/"1" (e.g. "close=2") fails validation, so it is treated as + * ordinary link text and absorbed into the link rather than parsed as the close flag. + */ + @Test + public void splitQuery_invalidCloseValue_staysInLink() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String link = "https://x.com/p?a=b&close=2"; + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + link; + Map q = invokeSplitQuery(url); + Assert.assertEquals(link, q.get("link")); + Assert.assertFalse(q.containsKey("close")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** + * Documents the fallback: a schemeless link fails link validation (no URI scheme), so the whole + * query falls back to the plain '&' split, which truncates a multi-param link. The server always + * prepends "https://", so this is an edge case; the test pins the current behavior. + */ + @Test + public void splitQuery_schemelessLink_fallbackTruncates() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=example.com/p?a=b&c=d"; + Map q = invokeSplitQuery(url); + Assert.assertEquals("example.com/p?a=b", q.get("link")); + Assert.assertEquals("d", q.get("c")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + // ===================== Close & Destroy Lifecycle ===================== /** @@ -895,4 +1310,179 @@ public void webView_usesApplicationContext_notActivity() { overlay.webView.getContext().getApplicationContext()); }); } + + // ===================== Plan verification (URL_EDGE_CASE_TESTS.md) — behavior gaps ===================== + // Empirically confirms plan behaviors that were not already covered by a dedicated test: actual + // event recording, malformed-payload safety, routing/parsing edge cases, and content-URL-handler + // fallthrough. (Scheme allow/deny decisions are covered in UtilsTests / CountlyWebViewClientTests / + // FeedbackDialogWebViewClientTests; percent-decoding in CountlyWebViewClientTests.) + + private void invokeStartSafeExternalIntent(String url) { + try { + Method m = ContentOverlayView.class.getDeclaredMethod("startSafeExternalIntent", String.class); + m.setAccessible(true); + m.invoke(overlay, url); + } catch (Exception e) { + Assert.fail("startSafeExternalIntent must not propagate an exception: " + e); + } + } + + /** §1.1 A valid event action actually records the event with its segmentation. */ + @Test + public void planEvent_validSingleEvent_isRecorded() { + EventProvider ep = TestUtils.setEventProviderToMock(Countly.sharedInstance(), mock(EventProvider.class)); + withActivity(activity -> { + overlay = createOverlay(activity); + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=event" + + "&event=[{\"key\":\"button_click\",\"sg\":{\"btn\":\"buy\"}}]"; + Assert.assertTrue(overlay.contentUrlAction(url, overlay.webView)); + }); + Map expected = new HashMap<>(); + expected.put("btn", "buy"); + TestUtils.validateRecordEventInternalMock(ep, "button_click", expected); + } + + /** §1.2 When an event carries both "sg" and "segmentation", "sg" wins. */ + @Test + public void planEvent_sgOverridesSegmentation_isRecorded() { + EventProvider ep = TestUtils.setEventProviderToMock(Countly.sharedInstance(), mock(EventProvider.class)); + withActivity(activity -> { + overlay = createOverlay(activity); + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=event" + + "&event=[{\"key\":\"e1\",\"sg\":{\"a\":\"1\"},\"segmentation\":{\"b\":\"2\"}}]"; + Assert.assertTrue(overlay.contentUrlAction(url, overlay.webView)); + }); + Map expected = new HashMap<>(); + expected.put("a", "1"); // taken from sg, "segmentation" ignored + TestUtils.validateRecordEventInternalMock(ep, "e1", expected); + } + + /** §1.4 An event object missing "key" records nothing (get("key") throws, caught) and does not crash. */ + @Test + public void planEvent_missingKey_recordsNothing() { + EventProvider ep = TestUtils.setEventProviderToMock(Countly.sharedInstance(), mock(EventProvider.class)); + withActivity(activity -> { + overlay = createOverlay(activity); + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=event&event=[{\"sg\":{\"a\":\"1\"}}]"; + Assert.assertTrue(overlay.contentUrlAction(url, overlay.webView)); + }); + TestUtils.validateRecordEventInternalMockInteractions(ep, 0); + } + + /** §5.4 The sentinel host appearing twice yields an empty parse (pairs != 2) -> not handled. */ + @Test + public void planRouting_sentinelTwice_emptyMapNotHandled() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=" + + Utils.COMM_URL + "/?x=1"; + Map q = invokeSplitQuery(url); + Assert.assertTrue("map must be empty when the sentinel appears twice", q.isEmpty()); + Assert.assertFalse(overlay.contentUrlAction(url, overlay.webView)); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** §5/§6 An extra path segment before the query fuses into the key, so the marker is unrecognized. */ + @Test + public void planRouting_extraPathSegment_markerNotRecognized() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String url = Utils.COMM_URL + "/foo?cly_widget_command=1&close=1"; + Map q = invokeSplitQuery(url); + Assert.assertNull("bare ?cly_widget_command must not be a key", q.get("?cly_widget_command")); + Assert.assertFalse(overlay.widgetUrlAction(url, overlay.webView)); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** §5.2 A query token with no '=' is skipped; surrounding params still parse. */ + @Test + public void planRouting_tokenWithoutEquals_skipped() { + withActivity(activity -> { + overlay = createOverlay(activity); + try { + String url = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&flagonly&link=https://example.com"; + Map q = invokeSplitQuery(url); + Assert.assertEquals("link", q.get("action")); + Assert.assertFalse("a bare token must not appear as a key", q.containsKey("flagonly")); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + } + + /** §5.5 On the content path the widget cancel runnable is never fired (only widgetUrlAction fires it). */ + @Test + public void planRouting_contentPathDoesNotFireWidgetCancel() { + AtomicBoolean cancelCalled = new AtomicBoolean(false); + AtomicBoolean closed = new AtomicBoolean(false); + withActivity(activity -> { + overlay = createOverlay(activity); + overlay.setOnWidgetCancelRunnable(() -> cancelCalled.set(true)); + String url = Utils.COMM_URL + "/?cly_x_action_event=1&cly_widget_command=1&close=1"; + Assert.assertTrue(overlay.contentUrlAction(url, overlay.webView)); + try { + closed.set((Boolean) getField("isClosed")); + } catch (Exception e) { + Assert.fail("read isClosed: " + e); + } + }); + Assert.assertTrue("overlay should be closed", closed.get()); + Assert.assertFalse("widget cancel must NOT fire on the content path", cancelCalled.get()); + } + + /** §6.10 A widget command value other than "1" is not handled. */ + @Test + public void planWidget_commandValueNotOne_returnsFalse() { + withActivity(activity -> { + overlay = createOverlay(activity); + String url = Utils.COMM_URL + "/?cly_widget_command=2&close=1"; + Assert.assertFalse(overlay.widgetUrlAction(url, overlay.webView)); + }); + } + + /** §6.16 Marker matching is case-sensitive: an uppercase command key is not recognized. */ + @Test + public void planWidget_uppercaseKey_returnsFalse() { + withActivity(activity -> { + overlay = createOverlay(activity); + String url = Utils.COMM_URL + "/?CLY_WIDGET_COMMAND=1&close=1"; + Assert.assertFalse(overlay.widgetUrlAction(url, overlay.webView)); + }); + } + + /** §8/handler A content URL handler returning false lets the SDK proceed; a denylisted scheme means + * no dispatch and no crash, and the handler is consulted exactly once. */ + @Test + public void planHandler_returnsFalse_fallsThroughNoCrash() { + AtomicBoolean called = new AtomicBoolean(false); + withActivity(activity -> { + ContentUrlHandler handler = url -> { + called.set(true); + return false; + }; + overlay = createOverlay(activity, null, null, handler); + invokeStartSafeExternalIntent("file:///blocked"); + }); + Assert.assertTrue("handler must be consulted", called.get()); + } + + /** §8/handler A throwing content URL handler is caught; startSafeExternalIntent must not propagate it. */ + @Test + public void planHandler_throws_caughtNoCrash() { + withActivity(activity -> { + ContentUrlHandler handler = url -> { + throw new RuntimeException("handler boom"); + }; + overlay = createOverlay(activity, null, null, handler); + invokeStartSafeExternalIntent("file:///blocked"); // reaching the next line proves it was caught + }); + } } diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/CountlyWebViewClientTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/CountlyWebViewClientTests.java index 2acd09d40..ef6a69c20 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/CountlyWebViewClientTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/CountlyWebViewClientTests.java @@ -102,6 +102,62 @@ private WebResourceResponse fakeHttpErrorResponse(int statusCode) { }; } + // ===================================== + // shouldOverrideUrlLoading - URL decoding + listener delivery + // ===================================== + + /** + * The URL is percent-decoded once before being handed to listeners, so encoded delimiters in + * an "event" JSON value (e.g. "%26" -> "&", "%5B" -> "[") arrive in plain form for parsing. + */ + @Test + public void shouldOverrideUrlLoading_decodesUrlForListener() { + final String[] received = new String[1]; + client.registerWebViewUrlListener((url, view) -> { + received[0] = url; + return true; + }); + + String encoded = Utils.COMM_URL + "/?cly_x_action_event=1&action=event&event=%5B%7B%22k%22%3A%22a%26b%22%7D%5D"; + String decoded = Utils.COMM_URL + "/?cly_x_action_event=1&action=event&event=[{\"k\":\"a&b\"}]"; + Assert.assertTrue(client.shouldOverrideUrlLoading(null, fakeRequest(encoded, true))); + Assert.assertEquals("listener must receive the decoded URL", decoded, received[0]); + } + + /** + * A literal '+' in a link is preserved (Uri.decode does not apply form '+'->space semantics), so + * deeplinks like "tel:+1..." and base64 query values are not corrupted. Matches iOS. + */ + @Test + public void shouldOverrideUrlLoading_plusInQuery_preserved() { + final String[] received = new String[1]; + client.registerWebViewUrlListener((url, view) -> { + received[0] = url; + return true; + }); + + String raw = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=https://x.com/search?q=a+b"; + Assert.assertTrue(client.shouldOverrideUrlLoading(null, fakeRequest(raw, true))); + Assert.assertEquals(raw, received[0]); + } + + /** + * A malformed percent-escape (possible inside an unencoded link) must not drop the action: the + * URL is decoded leniently and the listener is still invoked rather than the call returning false. + */ + @Test + public void shouldOverrideUrlLoading_malformedEscape_stillHandled() { + final String[] received = new String[1]; + client.registerWebViewUrlListener((url, view) -> { + received[0] = url; + return true; + }); + + String raw = Utils.COMM_URL + "/?cly_x_action_event=1&action=link&link=https://x.com?d=50%off"; + Assert.assertTrue(client.shouldOverrideUrlLoading(null, fakeRequest(raw, true))); + Assert.assertNotNull("listener must still be invoked on malformed escape", received[0]); + } + // ===================================== // onReceivedHttpError - abort logic // ===================================== diff --git a/sdk/src/main/java/ly/count/android/sdk/ContentOverlayView.java b/sdk/src/main/java/ly/count/android/sdk/ContentOverlayView.java index d885066fe..66d88f620 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ContentOverlayView.java +++ b/sdk/src/main/java/ly/count/android/sdk/ContentOverlayView.java @@ -782,12 +782,12 @@ boolean widgetUrlAction(String url, WebView view) { private void eventAction(Map query) { Log.i(Countly.TAG, "[ContentOverlayView] eventAction, event action detected"); - if (query.containsKey("event")) { - JSONArray event = (JSONArray) query.get("event"); - if (event == null) { - Log.w(Countly.TAG, "[ContentOverlayView] eventAction, event is null"); - return; - } + // splitQuery only stores "event" as a JSONArray when its JSON validates; a malformed payload + // from web content falls back to a raw String, so guard the cast (as resizeMeAction guards + // resize_me) to keep a ClassCastException from propagating out of shouldOverrideUrlLoading. + Object eventObj = query.get("event"); + if (eventObj instanceof JSONArray) { + JSONArray event = (JSONArray) eventObj; for (int i = 0; i < event.length(); i++) { try { JSONObject eventJson = event.getJSONObject(i); @@ -934,6 +934,22 @@ private void resizeContentInternal(@NonNull Activity activity) { } } + // Reserved content-communication param keys. Their names are reserved: a value (a link, or a + // segmentation string) that literally contains "&=" may be + // mis-split. This is documented for integrators. + private static final String[] RESERVED_KEYS = {"action", "event", "resize_me", "close", "link"}; + + // The whole action URL is already percent-decoded (CountlyWebViewClient), so values are in plain + // form. Two params can carry a literal '&' in their value: "link" (sent unencoded, may hold its + // own query string) and a decoded "event"/"resize_me" JSON (segmentation strings can contain + // '&'). A plain '&' split would therefore mis-slice them, and the params can appear in any order. + // + // Instead we span the query from the END: at each step we take the right-most reserved marker + // ("&=") whose value VALIDATES for that key (event/resize_me = JSON, close = 0/1, action = + // a known verb, link = has a URI scheme), record it, and shrink the span to its left. A marker + // whose value does NOT validate is treated as ordinary text inside an enclosing value (so it is + // skipped and absorbed by an outer param). What remains at the front is the comm-url-adjacent + // identifier ("?cly_x_action_event=1" / "?cly_widget_command=1"), parsed verbatim with its '?'. private Map splitQuery(@NonNull String url) { Map query_pairs = new HashMap<>(); String[] pairs = url.split(Utils.COMM_URL + "/?"); @@ -941,31 +957,107 @@ private Map splitQuery(@NonNull String url) { return query_pairs; } - String[] pairs2 = pairs[1].split("&"); - for (String pair : pairs2) { - int idx = pair.indexOf('='); - if (idx < 0) { - continue; + String q = pairs[1]; + int end = q.length(); + + while (end > 0) { + int chosenIdx = -1; + String chosenKey = null; + Object chosenValue = null; + + // Right-to-left, pick the first reserved marker whose value validates. Scanning from the + // right lets an inner (invalid) marker be absorbed into an outer, valid value. + int searchFrom = end; + while (searchFrom > 0) { + int marker = -1; + String markerKey = null; + for (String key : RESERVED_KEYS) { + int idx = q.lastIndexOf("&" + key + "=", searchFrom - 1); + if (idx > marker && idx + key.length() + 2 <= end) { + marker = idx; + markerKey = key; + } + } + if (marker < 0) { + break; + } + String value = q.substring(marker + markerKey.length() + 2, end); + Object parsed = validateReservedValue(markerKey, value); + if (parsed != null) { + chosenIdx = marker; + chosenKey = markerKey; + chosenValue = parsed; + break; + } + // Not a real param -> ordinary text; keep looking further left. + searchFrom = marker; } - String key = pair.substring(0, idx); - String value = pair.substring(idx + 1); - try { - if ("event".equals(key)) { - query_pairs.put(key, new JSONArray(value)); - } else if ("resize_me".equals(key)) { - query_pairs.put(key, new JSONObject(value)); - } else { - query_pairs.put(key, value); - } - } catch (JSONException e) { - Log.e(Countly.TAG, "[ContentOverlayView] splitQuery, Failed to parse JSON", e); + if (chosenIdx < 0) { + break; + } + query_pairs.put(chosenKey, chosenValue); + end = chosenIdx; + } + + // Remaining prefix is the identifier param(s) adjacent to the comm URL (leading '?' kept). + for (String pair : q.substring(0, end).split("&")) { + int idx = pair.indexOf('='); + if (idx >= 0) { + query_pairs.put(pair.substring(0, idx), pair.substring(idx + 1)); } } return query_pairs; } + // Validates a reserved param value and returns what to store, or null if it does not validate + // (meaning the "&=" was actually text inside an enclosing value, not a real parameter). + @Nullable + private Object validateReservedValue(@NonNull String key, @NonNull String value) { + switch (key) { + case "event": + try { + return new JSONArray(value); + } catch (JSONException e) { + return null; + } + case "resize_me": + try { + return new JSONObject(value); + } catch (JSONException e) { + return null; + } + case "close": + return "0".equals(value) || "1".equals(value) ? value : null; + case "action": + return "event".equals(value) || "link".equals(value) || "resize_me".equals(value) ? value : null; + case "link": + return hasUriScheme(value) ? value : null; + default: + return null; + } + } + + // True if value begins with a URI scheme (ALPHA *( ALPHA / DIGIT / "+" / "-" / "." ) ":"), which + // covers http(s) URLs and custom-scheme deeplinks. The server prepends "https://" to schemeless + // links, so a valid link value always carries a scheme. + private static boolean hasUriScheme(@NonNull String value) { + int colon = value.indexOf(':'); + if (colon <= 0) { + return false; + } + for (int i = 0; i < colon; i++) { + char c = value.charAt(i); + boolean ok = (i == 0) ? Character.isLetter(c) + : (Character.isLetterOrDigit(c) || c == '+' || c == '-' || c == '.'); + if (!ok) { + return false; + } + } + return true; + } + private void recalculateSafeAreaOffsets(@NonNull Activity activity) { SafeAreaDimensions safeArea = SafeAreaCalculator.calculateSafeAreaDimensions(activity, Countly.sharedInstance().L); diff --git a/sdk/src/main/java/ly/count/android/sdk/CountlyWebViewClient.java b/sdk/src/main/java/ly/count/android/sdk/CountlyWebViewClient.java index 639949862..1735f12d0 100644 --- a/sdk/src/main/java/ly/count/android/sdk/CountlyWebViewClient.java +++ b/sdk/src/main/java/ly/count/android/sdk/CountlyWebViewClient.java @@ -12,7 +12,6 @@ import android.webkit.WebResourceResponse; import android.webkit.WebView; import android.webkit.WebViewClient; -import java.net.URLDecoder; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; @@ -49,11 +48,14 @@ public CountlyWebViewClient(Set allowedSchemes) { public boolean shouldOverrideUrlLoading(WebView view, WebResourceRequest request) { String url = request.getUrl().toString(); Log.v(Countly.TAG, "[CountlyWebViewClient] shouldOverrideUrlLoading, url: [" + url + "]"); - try { - url = URLDecoder.decode(url, "UTF-8"); - } catch (Exception e) { - Log.e(Countly.TAG, "[CountlyWebViewClient] shouldOverrideUrlLoading, Failed to decode url", e); - return false; + + // Percent-decode with Uri.decode, NOT URLDecoder: URLDecoder applies form semantics and turns + // a literal '+' into a space, which corrupts links and deeplinks (e.g. "tel:+1..." or a base64 + // query value). Uri.decode decodes %XX, leaves '+' intact, and is lenient on a malformed + // escape (so the action is not dropped) - matching the iOS behavior. + String decoded = Uri.decode(url); + if (decoded != null) { + url = decoded; } Log.d(Countly.TAG, "[CountlyWebViewClient] shouldOverrideUrlLoading, urlDecoded: [" + url + "]");