Casting of `type="integer" values prevents uploads

[frafra]

Hi, it looks like there is a regression in SMASH, due to automatic casting of values to integers, while previous versions were sending values as a string to the server. This triggers an Uncaught exception in the old GSS server crash, but it might impact also users that do not rely on the old GSS server.

Here is a snippet of the server logs:

DEBUG:: /upload:: 2024-08-28 16:19:31:: /upload:: Received request from 10.255.0.2,
ACCESS:: ServletUtils:: 2024-08-28 16:19:31:: ServletUtils:: Connection from: Client 1 (/upload),
360303434 [qtp225672073-2517] WARN io.javalin.Javalin - Uncaught exception,
org.json.JSONException: JSONObject["value"] not a string.,
    at org.json.JSONObject.getString(JSONObject.java:721),
    at org.hortonmachine.gears.io.geopaparazzi.forms.Utilities.getImageIds(Utilities.java:344),
    at com.hydrologis.kukuratus.gss.GssServerApiJavalin.lambda$addClientUploadRoute$4(GssServerApiJavalin.java:367),
[...]

I found out that the client is storing integer as such, but the old GSS server expects all the value fields to be strings.

During the upload phase, the server tries to get the image IDs:

                        List<String> imageIds = Utilities.getImageIds(form);

-- https://github.com/geopaparazzi/GSS/blob/8c8fb151f2d8a08653e82c7e98e734d1420ad0c9/server_backend_java/src/main/java/com/hydrologis/kukuratus/gss/GssServerApiJavalin.java#L367

The Horton Machine goes through all the form items, and parses their values:

                    if (formItem.has(Utilities.TAG_VALUE))
                        value = formItem.getString(Utilities.TAG_VALUE);

-- https://github.com/TheHortonMachine/hortonmachine/blob/70a9625668eaaad7fa7a2d963c02ced522b7abca/gears/src/main/java/org/hortonmachine/gears/io/geopaparazzi/forms/Utilities.java#L343C1-L344C73

In order to reproduce the issue, one or more fields need to be defined with type="integer", for example:

          {
            "key": "count",
            "mandatory": "yes",
            "type": "integer",
            "value": "1"
          },

I inspected the GPAP file, and all the integers are stored as strings, so it looks like the conversion happens after having stored the data:

{
  "key": "count",
  "mandatory": "yes",
  "type": "integer",
  "value": "1"
}

This seems to have an effect on the server only in my case, but by looking at the existing issues, I found that using "type"="integer" might cause a crash in the client, thus I wonder if there is an underlying common problem related to undesired casting of integers to strings when reading the stored data: https://github.com/geopaparazzi/smash/issues/263

I am using SMASH 1.9.1 on iOS.

[frafra]

I made a fix by casting the integer back to string when intercepting the requests using mitmproxy, and it works: https://github.com/NINAnor/smash-hotfix.

[moovida]

Hi @frafra , sorry for the late reply. The fix here would need to be done on the old java server. The new should definitely not suffer from this, else it is a bug. Changing to the possibility of having numeric types has been a necessity of several contributing p(l)ayers and I think it is good to have it. The old server is no longer supported so the best thing would be to contribute there if you still have it in use. But you should definitely move to the django based :-)

[frafra]

Indeed :) The problem here is a regression in the behaviour of the client (even when using the old format), but it is understandable that the old mechanism is not supported any more :)

[frafra]

Just as side note: casting to string makes the server reply HTTP 200, but the server drops the data (no data is stored on the database). It looks that it too risky to use the old server. I guess would be safer to drop the option for using the old server from SMASH.