org.json.JSONException: Value null at lat of type org.json.JSONObject$1 cannot be converted to double

[CoelacanthusHex]

Describe the bug

java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:590)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:878)
Caused by: java.lang.reflect.InvocationTargetException
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:580)
    ... 1 more
Caused by: org.json.JSONException: Value null at lat of type org.json.JSONObject$1 cannot be converted to double
    at org.json.JSON.typeMismatch(JSON.java:101)
    at org.json.JSONObject.getDouble(JSONObject.java:456)
    at org.microg.gms.location.network.ichnaea.ExtensionsKt.toResponseLocation(extensions.kt:83)
    at org.microg.gms.location.network.ichnaea.ExtensionsKt.toGeolocateResponse(extensions.kt:39)
    at org.microg.gms.location.network.ichnaea.IchnaeaServiceClient$rawGeoLocate$2$2.onResponse(IchnaeaServiceClient.kt:104)
    at org.microg.gms.location.network.ichnaea.IchnaeaServiceClient$rawGeoLocate$2$2.onResponse(IchnaeaServiceClient.kt:103)
    at com.android.volley.toolbox.JsonRequest.deliverResponse(JsonRequest.java:100)
    at com.android.volley.ExecutorDelivery$ResponseDeliveryRunnable.run(ExecutorDelivery.java:102)
    at android.os.Handler.handleCallback(Handler.java:959)
    at android.os.Handler.dispatchMessage(Handler.java:100)
    at android.os.Looper.loopOnce(Looper.java:232)
    at android.os.Looper.loop(Looper.java:317)
    at android.app.ActivityThread.main(ActivityThread.java:8592)
    ... 3 more

To Reproduce

I don't know how to trigger this bug. But it happened only after I switch to https://beacondb.net as MLS provider.

Expected behavior

microG shouldn't crash when meet null as location lat. It's better to log and ignore it.

System Android Version: 14 Custom ROM: LineageOS 21

Additional context

https://beacondb.net seems to return HTTP 404 and a Json below when it can't detect location. It may be microG treat it as success response and try to find lat member in Json?

{"error":{"code":404,"errors":[{"domain":"geolocation","message":"No location could be estimated based on the data provided","reason":"notFound"}],"message":"Not found"}}

And the server implementation of BeaconDB is here: https://codeberg.org/beacondb/beacondb

[CoelacanthusHex]

This happened here: https://github.com/microg/GmsCore/blob/991da7bdd172add5eb664a4d713ca53513d5d5f3/play-services-location/core/provider/src/main/kotlin/org/microg/gms/location/network/ichnaea/extensions.kt#L38-L43 It was called from here: https://github.com/microg/GmsCore/blob/991da7bdd172add5eb664a4d713ca53513d5d5f3/play-services-location/core/provider/src/main/kotlin/org/microg/gms/location/network/ichnaea/IchnaeaServiceClient.kt#L101-L110 There is an ErrorListener for JSONObjectRequest, but why it wasn't invoked correctly?

[CoelacanthusHex]

According to BeaconDB's code, the only way to return a JSON with lat=null is when no location is detected, BeaconDB will return 404 with a JSON:

    Ok(HttpResponse::NotFound().json(json!(
        {
            "error": {
                "errors": [{
                    "domain": "geolocation",
                    "reason": "notFound",
                    "message": "No location could be estimated based on the data provided",
                }],
                "code": 404,
                "message": "Not found",
            }
        }
    )))

When a location is detected, it will return 200 with a JSON from struct LocationResponse:

return Ok(HttpResponse::Ok().json(LocationResponse));

And the definition of LocationResponse is:

#[derive(Debug, Serialize)]
struct LocationResponse {
    location: Location,
    accuracy: f64,
}

#[derive(Debug, Serialize)]
struct Location {
    lat: f64,
    lng: f64,
}

By the bound of the Rust type system, lat MUST be a valid IEEE 754 binary64 (double or f64 in programming languages). There is no possibility to return lat=null from this code path.

[Cryolitia]

I guess the result json value of JsonObjectRequest doesn't contains the accuracy field.

According to https://github.com/google/volley/blob/1.2.1/core/src/main/java/com/android/volley/toolbox/JsonObjectRequest.java#L91-L105 , the ErrorListener of JSONObjectRequest will be only called if and only if there's an unsupported encoding, or a Json deserialization exception occurred. No matter what the http response code it is.

[joelkoen]

beaconDB might be returning NaN, which could potentially be interpreted as a null value. I'll add logging to check if this is the case. @CoelacanthusHex , are you reliably able to get beaconDB to cause microG to crash?

[CoelacanthusHex]

Thanks @Cryolitia, we found the root cause of this issue: Volley's JSONObjectRequest treats all situations except unsupported encoding and JSON parse error as SUCCESS, whatever HTTP status code. https://github.com/google/volley/blob/67f7639b182b07deca07771bf98cf66b216baa12/core/src/main/java/com/android/volley/toolbox/JsonObjectRequest.java#L92-L105 So microG got the error message of BeaconDB as a success response and tried to find lat in it. Of course, microG will fail to find it. As MLS definition, when no location is found, the server should return 404 with an error message JSON: https://ichnaea.readthedocs.io/en/latest/api/geolocate.html#response In this way, microG shouldn't treat JSON as a successful response JSON when the HTTP status code is 404, which means this request has failed.

[CoelacanthusHex]

beaconDB might be returning NaN, which could potentially be interpreted as a null value.

Oh, this may be a possible reason as well.

I'll add logging to check if this is the case. @CoelacanthusHex , are you reliably able to get beaconDB to cause microG to crash?

I can't reproduce it. I rode a bicycle today, and when I returned home, I found microG crashed many times for the same reason in 4 minutes. Maybe I go through some areas where BeaconDB will return some response that microG can't handle correctly.

[joelkoen]

Based on my testing, I can confirm that microG handles 404 errors fine, but crashes when it receives NaN. This is definitely at least an issue in beaconDB.

[CoelacanthusHex]

Based on my testing, I can confirm that microG handles 404 errors fine, but crashes when it receives NaN. This is definitely an issue in beaconDB. codeberg.org/beacondb/beacondb/issues/9

Thanks for your investigation. I leave this issue open because I think microG shouldn't crash when meet null in response, whetever it's from.

[joelkoen]

Intercepting some requests from my phone, serde indeed serializes NaN as null. But yes, this should be handled instead of causing a crash.

{
    "accuracy": 50.0,
    "location": {
        "lat": null,
        "lng": null
    }
}

[Cryolitia]

Is microG using a Java deserializer in Kotlin? It usually breaks Kotlin' s null safety. kotlinx.serialization should be used instead.