WLED icon indicating copy to clipboard operation
WLED copied to clipboard

Fix presets.json corruption

Open calumr opened this issue 2 years ago • 3 comments

Handle whitespace before any opening '{'.

Fixes #2434

calumr avatar Dec 23 '22 15:12 calumr

This will not fix corruption. The corruption can also occur within preset itself. It just detects non space characters between preset objects. Which is good but I do not understand the concept you are aiming at.

blazoncek avatar Dec 23 '22 15:12 blazoncek

Sorry, you're right and I should have worded this better. It won't fix corruption, it will just stop it from occurring (for the case I have found).

When I was testing this locally bufferedFindObjectEnd was not returning the end of the object, so oldLen was always 1 in writeObjectToFile. With this fix oldLen is now correct.

calumr avatar Dec 23 '22 15:12 calumr

I explained it a bit more in the issue:

It's not handling the case where there is a space before the object begins:

"9": { "on": true, "bri": 120, ...

It gets called with f.position() pointing at the first <space> before the opening '{'. objDepth is set to 0 initially aso the while (count < bufsize) loop only gets run once.

calumr avatar Dec 23 '22 15:12 calumr

@blazoncek, @Aircoookie: should we merge this change, or not necessary? I don't know enough about file/json handling to decide what to do with this PR...

softhack007 avatar Jan 25 '23 19:01 softhack007

IDK. As we use ArduinoJson to create a JSON object in the file there is no reason for blanks to be inserted (check writeObjectToFileUsingId()and writeObjectToFile()).

This lets me believe that either a) @calumr pasted wrong JSON or b) ArduinoJson on his system/build behaves differently. By differently I mean it add spaces between keys and values which is very odd.

The proposed change is very subtle and should be rigorously tested before merging. I do not have time for such testing ATM.

blazoncek avatar Jan 25 '23 19:01 blazoncek

I've played a bit with this PR and was unable to reproduce insertion of spaces between preset ID and preset content by WLED alone.

But I did discover when such spaces may appear in presets.json, though. If you want to manually edit presets then minified version WLED stores is cumbersome and unwieldy so you are tempted to use some JSON prettify-er. If you then forget to minify presets and upload such pretty JSON to WLED it will function correctly but will fail upon modifying or saving new presets.

As such I deem this PR irrelevant as it only adds complexity in determining functionality of the function while not solving the problem entirely (spaces or tabs may be present elsewhere too). IMO a note in KB to minimize JSON after it has been manually edited should be sufficient as in-memory handling of presets.json to fix minimization after the upload to WLED is infeasible.

blazoncek avatar May 07 '23 08:05 blazoncek