square / moshi

A modern JSON library for Kotlin and Java.
https://square.github.io/moshi/1.x/
Apache License 2.0
9.78k stars 761 forks source link

Trailing commas #802

Open Shahroz16 opened 5 years ago

Shahroz16 commented 5 years ago

Thank you for a very helpful library. Is there any way to get past the trailing comma issue with moshi?

Sample json { "name": "Khan", "greeting": "Hello, how are you", }

If you can see there is an extra comma and moshi rightly sends out an exception. com.squareup.moshi.JsonEncodingException: Expected name at path $.greeting

Looking for any workaround for this issue. TIA

ZacSweers commented 5 years ago

I think that's an issue whatever service is sending you json right?

NightlyNexus commented 5 years ago

maybe we should make lenient readers read this?

Shahroz16 commented 5 years ago

@ZacSweers trailing commas are usually not allowed in Json, but since they are allowed in ECMA5 it would be really helpful if we have a feature of allowing it.

something along this Jackson trailing commas support

ZacSweers commented 5 years ago

ECMA5 is 10 years old, is that still relevant today?

Shahroz16 commented 5 years ago

JavaScript has allowed trailing commas in array literals since the beginning, and later added them to object literals (ECMAScript 5) and most recently (ECMAScript 2017) to function parameters. So trailing commas on browser works fine.

It's an edge case no doubt, but at times you can't dictate the 3rd party service providers and they expect mobile devices to be able to parse json which is being passed on browser.

But I get how it shouldn't be prioritized, so happy to wait on it.

On Fri, 22 Feb 2019, 3:28 am Zac Sweers, notifications@github.com wrote:

ECMA5 is 10 years old, is that still relevant today?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/square/moshi/issues/802#issuecomment-466193512, or mute the thread https://github.com/notifications/unsubscribe-auth/AEcWsipuF4JLen0y5qj7fVuzzwTMQ--wks5vPx2kgaJpZM4acKNG .

JakeWharton commented 5 years ago

I agree that leniency could handle this if it's easy but it should remain invalid by default.

swankjesse commented 4 years ago

Lenient mode handles this. Only problem is we treat the comma as an absent null like org.json.. This is probably not what you want.

killvetrov commented 4 years ago

So lenient mode should handle trailing commas now (in v1.9.3) or by Icebox milestone? I'm trying to parse this JSON:

{
    "id": 1,
    "watermark": {
        "x": 772,
        "y": 1130,
        "width": 221,
        "height": 70,
    }
}

Moshi throws JsonEncodingException even in lenient mode. I tried various ways: calling lenient() on adapter, making a JsonReader instance with setLenient(true) ending up with the same result. The thing is I don't even need watermark object, I only care about some other fields (I cut them out for brevity, minimum reproducible example). Does it mean that lenient mode only allows commas in arrays, but not in objects?

Also, what is best practice workaround for this (considering there is no control over JSON source)? Modifying adapter, or sanitizing JSON before reading (regexp)?

swankjesse commented 4 years ago

@killvetrov we’re strict about these currently, even in lenient mode. We should fix this.

Fintasys commented 6 months ago

There is an PR since last year, why isn't it being reviewed? This issue is quite annoying