Skip to content

Commit 393aa94

Browse files
rossbacherAndreas Rossbacher
andauthored
Fix a bug (and add a test) in the code that is handling empty (value) configurable path segments. In case a value for an configurable path segment is "" it is "invisible" in the url itself. 7.0 fixes a bug with handlung this but the way it worked that it did call matchUri recursively for all the children, but it should have only called it for the ones that actually are an configurable path segment match, as otherwise this will lead to mimatches. (#383)
Co-authored-by: Andreas Rossbacher <andreas.rossbacher@airbnb.com>
1 parent 0255e2f commit 393aa94

2 files changed

Lines changed: 103 additions & 4 deletions

File tree

deeplinkdispatch-base/src/main/java/com/airbnb/deeplinkdispatch/base/MatchIndex.java

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,10 +204,35 @@ childrenPos, getElementBoundaryPos(currentElementStartPosition),
204204
if (match == null && !pathSegmentReplacements.isEmpty()) {
205205
int childrenPos = getChildrenPos(currentElementStartPosition);
206206
if (childrenPos != -1) {
207-
match = matchUri(deeplinkUri, elements, placeholdersOutput,
208-
elementIndex,
209-
childrenPos, getElementBoundaryPos(currentElementStartPosition),
210-
pathSegmentReplacements);
207+
// Iterate through children and only recurse for empty configurable path segments
208+
int childStartPos = childrenPos;
209+
int parentBoundary = getElementBoundaryPos(currentElementStartPosition);
210+
211+
do {
212+
CompareResult childCompareResult = compareValue(
213+
childStartPos,
214+
urlElement.getTypeFlag(),
215+
urlElement.getValue(),
216+
pathSegmentReplacements
217+
);
218+
219+
// Only recurse for children that are empty configurable path segments
220+
if (childCompareResult != null
221+
&& childCompareResult.isEmptyConfigurablePathSegmentMatch()) {
222+
// Recurse into this child only (not its siblings)
223+
match = matchUri(deeplinkUri, elements, placeholdersOutput,
224+
elementIndex,
225+
childStartPos,
226+
getElementBoundaryPos(childStartPos),
227+
pathSegmentReplacements);
228+
229+
if (match != null) {
230+
break;
231+
}
232+
}
233+
234+
childStartPos = getNextElementStartPosition(childStartPos, parentBoundary);
235+
} while (childStartPos != -1);
211236
}
212237
}
213238
}
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package com.airbnb.deeplinkdispatch
2+
3+
import com.airbnb.deeplinkdispatch.base.Utils.toByteArrayMap
4+
import org.assertj.core.api.Assertions.assertThat
5+
import org.junit.Test
6+
7+
/**
8+
* Test for a bug where URLs with fewer path segments incorrectly match patterns
9+
* expecting more segments when configurablePathSegmentReplacements is non-empty.
10+
*/
11+
@kotlin.ExperimentalUnsignedTypes
12+
class ConfigurablePathSegmentMatchingTest {
13+
/**
14+
* When configurablePathSegmentReplacements is non-empty, a URL with 4 path
15+
* parameter values should match a 4-parameter pattern, NOT a 5-parameter pattern.
16+
*
17+
* Bug behavior: The 5-param pattern matches with the last URL value ("v4")
18+
* duplicated into both placeholder_4 and placeholder_5 parameters.
19+
*/
20+
@Test
21+
fun `URL with 4 values should match 4-param pattern not 5-param pattern`() {
22+
val pattern4Params = activityDeepLinkEntry("test://host/path/{placeholder_1}/{placeholder_2}/{placeholder_3}/{placeholder_6}")
23+
val pattern5Params =
24+
activityDeepLinkEntry("test://host/path/{placeholder_1}/{placeholder_2}/{placeholder_3}/{placeholder_4}/{placeholder_5}")
25+
26+
val root = Root()
27+
root.addToTrie(pattern5Params)
28+
root.addToTrie(pattern4Params)
29+
val testRegistry = TestRegistry(root.toUByteArray().toByteArray())
30+
31+
// URL with exactly 4 placeholder values
32+
val testUrl = "test://host/path/id123/details/amenities/add"
33+
34+
// Non-empty configurablePathSegmentReplacements triggers the bug
35+
val pathSegmentReplacements = toByteArrayMap(mapOf("configurable_path_segment" to ""))
36+
37+
val match = testRegistry.idxMatch(DeepLinkUri.parse(testUrl), pathSegmentReplacements)
38+
39+
assertThat(match).isNotNull
40+
41+
// Should match 4-param pattern
42+
assertThat(match!!.deeplinkEntry.uriTemplate)
43+
.describedAs("URL with 4 values should match 4-param pattern, not 5-param pattern")
44+
.isEqualTo(pattern4Params.uriTemplate)
45+
46+
val parameters = match.getParameters(DeepLinkUri.parse(testUrl))
47+
48+
// Should have exactly 4 parameters
49+
assertThat(parameters)
50+
.describedAs("Should have exactly 4 parameters")
51+
.hasSize(4)
52+
53+
// Verify correct parameter values
54+
assertThat(parameters["placeholder_1"]).isEqualTo("id123")
55+
assertThat(parameters["placeholder_2"]).isEqualTo("details")
56+
assertThat(parameters["placeholder_3"]).isEqualTo("amenities")
57+
assertThat(parameters["placeholder_6"]).isEqualTo("add")
58+
59+
// Should NOT have parameters from the 5-param pattern
60+
assertThat(parameters)
61+
.describedAs("Should not contain placeholder_4 from the 5-param pattern")
62+
.doesNotContainKey("placeholder_4")
63+
assertThat(parameters)
64+
.describedAs("Should not contain placeholder_5 from the 5-param pattern")
65+
.doesNotContainKey("placeholder_5")
66+
}
67+
68+
companion object {
69+
private fun activityDeepLinkEntry(
70+
uriTemplate: String,
71+
className: String = "com.example.TestActivity",
72+
): DeepLinkEntry = DeepLinkEntry.ActivityDeeplinkEntry(uriTemplate, className)
73+
}
74+
}

0 commit comments

Comments
 (0)