insanum / sncli

Simplenote CLI
MIT License
396 stars 34 forks source link

Notes synced with Epoch time issues cause sncli to crash #71

Closed citizenkepler closed 5 years ago

citizenkepler commented 5 years ago

When performing my initial sync I got a year out of range message. Thinking this was odd, I investigated:

citizen@genciti ~ $ sncli sncli database doesn't exist, forcing full sync... Starting full sync Synced new note from server (key=beea78d7a10711e282fd8ddac0098ab0) [omited output] Synced new note from server (key=66334199707411e2ad87ab0bbc69d078) Saved note to disk (key=beea78d7a10711e282fd8ddac0098ab0) [omited output] Saved note to disk (key=66334199707411e2ad87ab0bbc69d078) Full sync completed Traceback (most recent call last): File "/usr/bin/sncli", line 11, in load_entry_point('sncli==0.3.0', 'console_scripts', 'sncli')() File "/home/citizen/.local/lib64/python3.6/site-packages/simplenote_cli/sncli.py", line 1353, in main sncli(sync, verbose, config).gui(key) File "/home/citizen/.local/lib64/python3.6/site-packages/simplenote_cli/sncli.py", line 960, in gui 'log' : self.log File "/home/citizen/.local/lib64/python3.6/site-packages/simplenote_cli/view_titles.py", line 18, in init urwid.SimpleFocusListWalker(self.get_note_titles())) File "/home/citizen/.local/lib64/python3.6/site-packages/simplenote_cli/view_titles.py", line 143, in get_note_titles lines.append(self.get_note_title(n.note)) File "/home/citizen/.local/lib64/python3.6/site-packages/simplenote_cli/view_titles.py", line 128, in get_note_title return urwid.AttrMap(self.format_title(note), File "/home/citizen/.local/lib64/python3.6/site-packages/simplenote_cli/view_titles.py", line 56, in format_title dt = datetime.datetime.fromtimestamp(time.mktime(t)) ValueError: year 45324 is out of range citizen@genciti ~ $

After digging through a lot of my notes looking for the issue as a blank account worked without issue, I found a few of my notes have a decimal in the epoch timestamp. Looking at view_titles.py, the modification time is the timestamp being looked at when the error occurs.

I tracked the issue down to this specific note, below I created a blank .sncli directory with this single problematic note:

citizen@genciti ~ $ sudo /etc/init.d/net.eno1 stop Bringing down interface eno1 citizen@genciti ~ $ ls -l .sncli total 4 -rw-r--r-- 1 citizen citizen 372 Oct 5 12:23 0190e763b8e411e2817c53cebaffd385.json citizen@genciti ~ $ cat .sncli/0190e763b8e411e2817c53cebaffd385.json { "tags": [], "deleted": false, "shareURL": "", "publishURL": "", "content": "Camping\n\nDevils river state nateral area", "systemTags": [], "modificationDate": 1368130053817.0, "creationDate": 1368130019000.0, "key": "0190e763b8e411e2817c53cebaffd385", "version": 2, "syncdate": 1538767151.3569057, "localkey": "0190e763b8e411e2817c53cebaffd385" }citizen@genciti ~ $ citizen@genciti ~ $ sncli Traceback (most recent call last): File "/usr/bin/sncli", line 11, in load_entry_point('sncli==0.3.0', 'console_scripts', 'sncli')() File "/home/citizen/.local/lib64/python3.6/site-packages/simplenote_cli/sncli.py", line 1353, in main sncli(sync, verbose, config).gui(key) File "/home/citizen/.local/lib64/python3.6/site-packages/simplenote_cli/sncli.py", line 960, in gui 'log' : self.log File "/home/citizen/.local/lib64/python3.6/site-packages/simplenote_cli/view_titles.py", line 18, in init urwid.SimpleFocusListWalker(self.get_note_titles())) File "/home/citizen/.local/lib64/python3.6/site-packages/simplenote_cli/view_titles.py", line 144, in get_note_titles lines.append(self.get_note_title(n.note)) File "/home/citizen/.local/lib64/python3.6/site-packages/simplenote_cli/view_titles.py", line 129, in get_note_title return urwid.AttrMap(self.format_title(note), File "/home/citizen/.local/lib64/python3.6/site-packages/simplenote_cli/view_titles.py", line 57, in format_title dt = datetime.datetime.fromtimestamp(time.mktime(t)) ValueError: year 45324 is out of range citizen@genciti ~ $

This issue should replicate with the following note: { "tags": [], "deleted": false, "shareURL": "", "publishURL": "", "content": "Camping\n\nDevils river state nateral area", "systemTags": [], "modificationDate": 1368130053817.0, "creationDate": 1368130019000.0, "key": "0190e763b8e411e2817c53cebaffd385", "version": 2, "syncdate": 1538767151.3569057, "localkey": "0190e763b8e411e2817c53cebaffd385" }

While the date is out of range the other clients in my symplenote ecosystem (Offical Simplenote Client, Notational Volicaty) hadle this note gracefully.

samuelallan72 commented 5 years ago

Thanks for the detailed report! I am curious though how the date could get so much out of range. I had hoped that this would be validated server-side.

I feel a bit conflicted about this - the question is now which invalid data cases should be rejected, silently sanitized, or cause a crash due to irrecoverable errors. I don't think the current note parsing is very robust in this way, relying on the user to always feed it sane input. There are also many places where the data could be modified/sourced, including the simplenote server, interactive user input in sncli, raw importing of json note data, and potential for the json files on disk to be edited externally...

In saying this, it would seem that sncli should be robust in how it handles data synced from the server, since that data could have been supplied by other applications. Other data sources are likely to be controlled more directly by the user and thus shouldn't require such sanitizing?

Open to suggestions. :)

citizenkepler commented 5 years ago

After filling this and identifying the field causing the issue, I manually re-created notes affected by this issue. I think these notes were all created a few years ago by a bad android simple note client.

That said I am not sure how much of an edge case this is. I am with you that it's weird that the server allows creation or modification dates in the future...

I would recommend if the modification datestamp is in the future by more than X in the future that you quilt and let the user know the note's title. That's what I basically what I did by added an extra debugging print statement showing the current note being parsed before the crash to debug the notes causing the crash and re-create them.

samuelallan72 commented 5 years ago

Looking into making a fix for this now. It seems the new simplenote api does not allow pushing a note with modification date set far in the future, so this shouldn't be an issue from now. I like your idea for fixing the date if it is insanely in the future. :+1: