Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ internal class DeepLinkMatcher<T : NavKey>(
// match queries (if any)
request.queries.forEach { query ->
val name = query.key
val queryStringParser = deepLinkPattern.queryValueParsers[name]
val queryParsedValue = try {
queryStringParser!!.invoke(query.value)
val queryParameter = deepLinkPattern.queryParameters[name]!!
val (argName, parsedValue) = try {
Pair(queryParameter.serialName ?: name, queryParameter.typeParser.invoke(query.value))
} catch (e: IllegalArgumentException) {
Log.e(TAG_LOG_ERROR, "Failed to parse query name:[$name] value:[${query.value}].", e)
return null
}
args[name] = queryParsedValue
args[argName] = parsedValue
}
Comment on lines 49 to 59
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The use of the non-null assertion operator (!!) on line 51 can cause a NullPointerException if the request URI contains a query parameter that is not defined in the deep link pattern. This would crash the application. It's safer to handle this case by treating it as a non-match, which is consistent with how path segments are handled. Using a for loop instead of forEach allows for an early return from the match function if an unknown parameter is found.

Suggested change
request.queries.forEach { query ->
val name = query.key
val queryStringParser = deepLinkPattern.queryValueParsers[name]
val queryParsedValue = try {
queryStringParser!!.invoke(query.value)
val queryParameter = deepLinkPattern.queryParameters[name]!!
val (argName, parsedValue) = try {
Pair(queryParameter.serialName ?: name, queryParameter.typeParser.invoke(query.value))
} catch (e: IllegalArgumentException) {
Log.e(TAG_LOG_ERROR, "Failed to parse query name:[$name] value:[${query.value}].", e)
return null
}
args[name] = queryParsedValue
args[argName] = parsedValue
}
for (query in request.queries) {
val name = query.key
val queryParameter = deepLinkPattern.queryParameters[name] ?: return null
val (argName, parsedValue) = try {
Pair(queryParameter.serialName ?: name, queryParameter.typeParser.invoke(query.value))
} catch (e: IllegalArgumentException) {
Log.e(TAG_LOG_ERROR, "Failed to parse query name:[$name] value:[${query.value}].", e)
return null
}
args[argName] = parsedValue
}

// provide the serializer of the matching key and map of arg names to parsed arg values
return DeepLinkMatchResult(deepLinkPattern.serializer, args)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,16 @@ internal class DeepLinkPattern<T : NavKey>(
}

/**
* Parse supported queries into a map of queryParameterNames to [TypeParser]
* Parse supported queries into a map of queryParameterNames to [QueryParameter]
*
* This will be used later on to parse a provided query value into the correct KType
*/
val queryValueParsers: Map<String, TypeParser> = buildMap {
val queryParameters: Map<String, QueryParameter> = buildMap {
uriPattern.queryParameterNames.forEach { paramName ->
val elementIndex = serializer.descriptor.getElementIndex(paramName)
val paramValue = uriPattern.getQueryParameter(paramName)?.removePrefix("{")?.removeSuffix("}")
val elementIndex = serializer.descriptor.getElementIndex(paramValue ?: paramName)
val elementDescriptor = serializer.descriptor.getElementDescriptor(elementIndex)
this[paramName] = getTypeParser(elementDescriptor.kind)
this[paramName] = QueryParameter(paramValue, getTypeParser(elementDescriptor.kind))
}
}

Expand All @@ -92,6 +93,14 @@ internal class DeepLinkPattern<T : NavKey>(
val isParamArg: Boolean,
val typeParser: TypeParser
)

/**
* Metadata about a supported query parameter
*/
class QueryParameter(
val serialName: String?,
val typeParser: TypeParser,
)
Comment on lines +100 to +103
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For classes that primarily hold data, like QueryParameter, it's a good practice to declare them as a data class. This automatically generates useful methods like equals(), hashCode(), toString(), and copy(), which improves code conciseness and can be helpful for debugging.

Suggested change
class QueryParameter(
val serialName: String?,
val typeParser: TypeParser,
)
data class QueryParameter(
val serialName: String?,
val typeParser: TypeParser,
)

}

/**
Expand Down