929861 - bookmark backup JSON file format not up to spec

Status: RESOLVED WORKSFORME

(Firefox :: Bookmarks & History, defect)

Description

[Jim Michaels]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 20130910160258

Steps to reproduce:

1.backed up the bookmarks to a .json file from ff 24.0
2.opened the json file in notepad++
3.installed the json viewer plugin using the plugin manager
4.ctrl-a
5.plugins, JSON viewer, format JSON
6.examine bracket matching for end items (actually, you can do this without steps 3-5




Actual results:

json file is missing at least the ending ]} maybe more. I don't know if there is supposed to be more entries parallel to the main entry or not. if there is, they are missing. it is not following the JSON file format.

I also don't know if there is supposed to be any more values between ] and }



Expected results:

- should have at least ]} at end
- the json importer must be able to handle older incorrect json file formats with the mistake in it, for compatibility, so people don't lose their bookmarks, and the correct one.

Comment 1

[Jeff Walden [:Waldo]]

As far as I know, we haven't serialized incorrect JSON to file in years -- since 3.0 or 3.5 at least.  We carried around support for parsing that incorrect JSON for a near-equivalent amount of time -- til past the time we supported updating directly from that last affected version to the latest version.

I'm not aware of any instances of us serializing non-JSON data since that time -- certainly not in 24.  And we're serializing exactly a string returned by JSON.stringify, which is almost by definition valid JSON.  Is there any chance at all that your viewer might be screwing up its own JSON parsing?  Perhaps messing up escapes or something?

Comment 2

[Jeff Walden [:Waldo]]

I might have touched the JSON-ful bits of this, but I think Marco is the one most responsible for it on an ongoing basis.  Setting neeinfo? so he can pick up the torch here (feel free to throw back if there *is* some sort of actual JS engine issue implicated here).

Comment 3

[Jim Michaels]

that being the case that it's JS actually generating the JSON, then I would say that it's the js engine that's broke too. 

the side effects of this bug is that sites which have been relying on exporting JSON data such as AJAX libraries like jquery (specifically using JSON calls), anything with ff that relies on that JSON as a database would have corrupted data if it is not updated or corrected manually in the switchover. there would be browser upgrade timing "echo-ripple" effects as well, since people upgrade their browsers at different times. given 1 server and people with different states of browser-upgraded-ness, would have interesting effects if the browser were allowed to do exports. and JSON is popular right now. having the ability to read broken and fixed format would solve the problem for ff users. other browsers in this situation I do not know how they handle such a situation. this could also present interesting needs to present upgrades to code maybe for importing browser bookmarks for other browsers, IF they use ff JSON files and not the SQLITE db's (zat what dey use?).

I only use JSON as something to import data, but someone who exports and imports data both, this could be a problem. and you might see some bug reports relating to that fact.

I have no idea if this affects any e-cart software for web sites, since I have not seen their code. but these are the implications.

on the other hand, exporting broken json structures from ff and importing into some other browser which may do it correctly would have interesting results... and this may fix some long-standing bugs which people have been dealing with.

personally? I would like to see this fixed.

Comment 4

[Jeff Walden [:Waldo]]

(In reply to Jim Michaels from comment #3)
> that being the case that it's JS actually generating the JSON, then I would
> say that it's the js engine that's broke too. 
>
> personally? I would like to see this fixed.

Supposing there's an actual issue in our JSON generation code, so would I.  :-)

But we haven't had reports of JSON generation going awry like this that I'm aware of.  For it to *only* have appeared via bookmarks backup, seems implausible.  So I suspect the issue isn't with that.

Out of curiosity, what's the exact size of the generated JSON file?  I wonder if output might possibly have been truncated or something.

Comment 5

[Marco Bonardo [:mak]]

I think it's related to the changes we did recently to bookmarks backups code, not to the json stringification code. While I don't have a proof of this, I'm 90% sure it's the case.

Comment 6

[Jim Michaels]

I don't think size will tell you anything, since the structures contain variable-length content, and there can be any number of records (just from looking at the content). if they were fixed-size records, then you could calculate the correct size of the JSON db.

1,946,342 bookmarks-2013-10-24.json

Comment 7

[Jeff Walden [:Waldo]]

The truncation I was wondering about would likely have been by the file-writing abstractions in use, and would have shown up if the size were some sort of power of two (or sum of two of them, or some other glaringly obviously weird binary pattern).  1946342 doesn't fit that at all, so it appears my guess didn't pan out.  :-)

Comment 8

[Marco Bonardo [:mak]]

I think bug 824433 is going to resolve this, since it will one-shot stringify the object.

Comment 9

[Marco Bonardo [:mak]]

I think this should be working properly with the latest changes, please verify with the latest nightly whether you still notice issues with the format of the newest backup.

Comment 10

[Jim Michaels]

ftp://ftp.mozilla.org/pub/firefox/nightly/2014/02/2014-02-09-03-45-01-mozilla-b2g28_v1_3/
passed my brace checker program on my 1.9MB bookmarks. should I be using a different nightly? please be specific there are many files on the ftp server, and I don't know what the funny names are (aurora, 1.2 1.3, etc). if I choose the nightly 1.3 ff28, I get what I am using when I do a normal update through help, about I think. thanks.

notepad++ JSON Viewer plugin currently has a formatting bug when it formats JSON.

compiling this c++ JSON validator: https://github.com/terryjsmith/c-json-validator
and feeding JSON through it I get a successful parse.

as for the first validator I tried, I could not use my 1.9MB JSON file with jsonlint.com since ff continually asks to stop the script (never quits when I hit stop). (and besides, 2MB probably would probably make a huge syntax parse tree).

Comment 11

[Marco Bonardo [:mak]]

I meant http://nightly.mozilla.org

Btw, if I read your comment correctly you got a successful parse, so I'd call this worksforme

Comment 12

[Jim Michaels]

I believe I got a successful parse, yes.