Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RouteOptions::toUrl doesn't encode '&' character in query parameters #1409

Closed
tomaszrybakiewicz opened this issue Apr 20, 2022 · 2 comments · Fixed by #1433
Closed

RouteOptions::toUrl doesn't encode '&' character in query parameters #1409

tomaszrybakiewicz opened this issue Apr 20, 2022 · 2 comments · Fixed by #1433
Assignees
Labels

Comments

@tomaszrybakiewicz
Copy link
Contributor

tomaszrybakiewicz commented Apr 20, 2022

Description

One of our customers detected a crash caused by requesting routes with waypoint names containing '&' character. From the captured crash log we determined the root cause of the crash to be incorrect waypoint_names argument encoding by the RouteOptions::toUrl method.

DIRECTIONS_API?access_token=<token>&geometries=polyline6&alternatives=true&overview=full&steps=true&avoid_maneuver_radius=111.11111450195312&bearings=13.6232796,90;&layers=0;&continue_straight=true&annotations=distance,congestion,maxspeed&language=en_SG&roundabout_exits=true&voice_instructions=true&banner_instructions=true&voice_units=metric&waypoints=0;1&waypoint_names=;Serangoon%20Garden%20Market%20&%20Food%20Centre&enable_refresh=true

Crash details

Investigation and support ticket: https://github.com/mapbox/ask-navigation/issues/94

Configuration:
Operating systems + versions: Android 11
Device/simulator/emulator models: Samsung Galaxy Note20 Ultra 5G

Steps to reproduce

  1. set a destination that contains & in the name of destination
val waypointNamesList=listof("","Serangoon Garden Market & Food Centre")
var routeOptionsBuilder = RouteOptions.builder()
....
routeOptionsBuilder.waypointNamesList(waypointNamesList)
routeOptionsBuilder.build()
  1. make reroute happen

Captured Crash log:

Fatal Exception: java.lang.StringIndexOutOfBoundsException: String index out of range: -1
       at java.lang.String.substring(String.java:2064)
       at com.mapbox.api.directions.v5.models.RouteOptions.fromUrl(RouteOptions.java:883)
       at com.mapbox.navigation.base.route.NavigationRoute$Companion.create(NavigationRoute.java:71)
       at com.mapbox.navigation.base.internal.utils.DirectionsResponseUtilsKt$parseDirectionsResponse$2.invokeSuspend(DirectionsResponseUtils.kt:19)
       at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
       at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
       at java.lang.Thread.run(Thread.java:923)
@jinny-nam
Copy link

hi @korshaknn I see the PR has been merged, but it seems that we are going to need a better solution? (based on your description on the PR)
On the other hand, the customer would like to have a patch for the fix, other than waiting for the next sdk release that includes the fix. Would it be a possible option?
cc: @linsd75

@korshaknn
Copy link
Contributor

@jinny-nam hey! yes, we are going to provide a patch.
cc @LukasPaczos @zugaldia

@LukasPaczos LukasPaczos changed the title RouteOptions::toUrl incorrectly encodes waypoint_names that include '&' character. RouteOptions::toUrl doesn't encode '&' character in query parameters Apr 22, 2022
@LukasPaczos LukasPaczos assigned LukasPaczos and unassigned korshaknn Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants