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

JSON decoding fails on trailing comma #16945

Closed
kvark opened this issue Sep 2, 2014 · 8 comments
Closed

JSON decoding fails on trailing comma #16945

kvark opened this issue Sep 2, 2014 · 8 comments

Comments

@kvark
Copy link
Contributor

@kvark kvark commented Sep 2, 2014

extern crate debug;
extern crate serialize;
use serialize::json;

#[deriving(Decodable)]
struct Spec {
    pub schema: String,
}

static INPUT: &'static str = "
{
    \"schema\" : \"xxx\",
}
";

fn main() {
    println!("{:?}", json::decode::<Spec>(INPUT));
}

Gives me:

task '

' failed at 'assertion failed: !self.is_empty()', /build/rust-git/src/rust/src/libserialize/json.rs:1206

@steveklabnik
Copy link
Member

@steveklabnik steveklabnik commented Sep 2, 2014

That's because JSON doesn't allow for a trailing comma, I'd imagine.

@mahkoh
Copy link
Contributor

@mahkoh mahkoh commented Sep 2, 2014

The standard specification is clearly wrong here and the decoder should accept trailing commas.

@steveklabnik
Copy link
Member

@steveklabnik steveklabnik commented Sep 2, 2014

Well, that's not how standards work.

@mahkoh
Copy link
Contributor

@mahkoh mahkoh commented Sep 2, 2014

Not sure what you mean by that. I'm not saying that the encoder should produce trailing commas.

@mahkoh
Copy link
Contributor

@mahkoh mahkoh commented Sep 2, 2014

There is some precedence for not allowing trailing commas: Go, Chromium, and Firefox reject such json. The specification is still wrong for not allowing them and I see no reason not to parse them.

@kvark
Copy link
Contributor Author

@kvark kvark commented Sep 2, 2014

@steveklabnik sad but true. Please feel free to close the issue then.

@kvark
Copy link
Contributor Author

@kvark kvark commented Sep 2, 2014

We could, however, have a better error message.

@lucy
Copy link
Contributor

@lucy lucy commented Sep 2, 2014

It shouldn't fail, since it returns a Result containing possible parse errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants