1685486 - SessionState.fromString() throws JSONException (typeMismatch)

Status: RESOLVED FIXED

(GeckoView :: General, defect, P1)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

Related AC Issue: https://github.com/mozilla-mobile/android-components/issues/9332

We recently migrated our session restore code to use JsonReader instead of org.json (to parse from a stream instead of loading everything into memory as string first). With that we stopped catching JSONException since that was only getting thrown by the org.json implemenetation.

However with that we then got crashes and saw that JSONException also gets thrown out of GeckoSession$SessionState.fromString:

org.json.JSONException
	at org.json.JSON.typeMismatch(JSON.java:112)
	at org.json.JSONObject.<init>(JSONObject.java:168)
	at org.json.JSONObject.<init>(JSONObject.java:181)
	at org.mozilla.geckoview.GeckoSession$SessionState.fromString(GeckoSession.java:1)

Looking a bit into the code I noticed that sometimes toString() returns null:
https://searchfox.org/mozilla-central/rev/ef900cd2258d4c5d968093f612f807d96e6e7c98/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java#2219-2234

But fromString() doesn't check and always tries to parse as JSONObject:
https://searchfox.org/mozilla-central/rev/ef900cd2258d4c5d968093f612f807d96e6e7c98/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java#2214-2216

• fromString() should handle null values.
• I guess ideally fromString() would never throw JSONException since that is an implementation detail.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

In the meantime we started catching JSONException in AC again to work around the startup crashes that we have been seeing.

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

The stacktrace above has the exception message stripped (by Socorro). In a full stacktrace it shows up as:

01-06 13:33:58.883 22743 22743 E ExceptionHandler: org.json.JSONException: Value null of type org.json.JSONObject$1 cannot be converted to JSONObject
01-06 13:33:58.883 22743 22743 E ExceptionHandler: at org.json.JSON.typeMismatch(JSON.java:111)

Comment 3

[Eugen Sawin [:esawin]]

Are you going to look into this?

Comment 4

[Dylan Roeh (:droeh) (he/him)]

Attachment: Bug 1685486 - Annotate SessionState.fromString/toString appropriately, stop throwing JSONExceptions, and handle null values a bit more gracefully. r=#geckoview-reviewers!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf692733d4f6
Annotate SessionState.fromString/toString appropriately, stop throwing JSONExceptions, and handle null values a bit more gracefully. r=agi,geckoview-reviewers,owlish

Comment 6

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/cf692733d4f6