Skip to content
This repository has been archived by the owner on Mar 5, 2024. It is now read-only.

Backup playerlist in event of malformed json #235

Closed
DiegoFleitas opened this issue Dec 30, 2020 · 7 comments
Closed

Backup playerlist in event of malformed json #235

DiegoFleitas opened this issue Dec 30, 2020 · 7 comments
Labels
Priority: Medium This issue may be useful, and needs some attention. Type: Enhancement New feature or request
Milestone

Comments

@DiegoFleitas
Copy link

Describe the bug

A clear and concise description of what the bug is.
Manually modifying the playerlist.json file ended up with the file being overwriten by the software.

To Reproduce

Steps to reproduce the behavior:
I no longer have the file to check what triggered it

Expected behavior

A clear and concise description of what you expected to happen.
It shouldnt load the playerlist in question, completely overwriting a file is terrible behavior, this file was over 2k long and I have no backups...

Logs

Add any relevant logs. Logs are found by going to File → Open Logs Folder
ConfigHelpers.cpp(228):LoadFileInternal: Failed to parse JSON from "cfg/playerlist.json": class nlohmann::detail::parse_error: [json.exception.parse_error.101] parse error at line 3300, column 2: syntax error while parsing value - unexpected ']'; expected '[', '{', or a literal

Screenshots

If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: Windows 10
  • Version of bot detector 1.1.0.674
@DiegoFleitas DiegoFleitas added Priority: Medium This issue may be useful, and needs some attention. Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. labels Dec 30, 2020
@DiegoFleitas
Copy link
Author

I think the priority for this should be high too

@ClusterConsultant
Copy link
Collaborator

I think this is somewhat intended behavior. In the event of a malformed json input it will overwrite the offending file following the json scheme. In general it is strongly recommended to backup files you are modifying and also to use a linter when working with json. Some provision to prevent this could be baked in but I am not sure how valuable that would be when users can do it so effortlessly.

@DiegoFleitas
Copy link
Author

DiegoFleitas commented Dec 30, 2020

No, this is a wrong. A valid JSON is allowed to have a , at the last item and currently having so deletes the whole JSON. The current ruling that causes the problem is validated by JSON linters

@DiegoFleitas
Copy link
Author

I'm sorry, upon studying the issue it seems i was in the wrong here. JSON doesnt actually allow trailing commas but parsers often fix that silently. I still think it would be good to have anyways

@ClusterConsultant
Copy link
Collaborator

ClusterConsultant commented Dec 30, 2020

After trying 3 different linters, they all agree. That is an EOF error. I am fairly certain that valid json does not end in a comma.

edit: this was sent at the same time as the above message

@DiegoFleitas
Copy link
Author

Indeed, the linter I was using was fixing it for me https://jsonformatter.curiousconcept.com/

@PazerOP
Copy link
Owner

PazerOP commented Dec 30, 2020

Hey there, sorry about that. It is indeed intended behavior that it overwrites config files if it is unable to parse them, and replace them with the default config. There was code in there to make a backup of the file, but that only applied to settings.json. Trailing comma support is unlikely, since unfortunately the author of the json library I am using has a fairly extreme (and in my opinion unhelpful) stance on adding stuff that isn't in the json spec nlohmann/json#376.

I already need to take a look at the config file code to address some other robustness issues, so I'll see if I can add support for making backup files for anything that gets overwritten at the same time.

@ClusterConsultant ClusterConsultant changed the title [BUG] Entire playerlist deleted [Enhancment] Backup playerlist in event of malformed json Dec 30, 2020
@ClusterConsultant ClusterConsultant added Type: Enhancement New feature or request and removed Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. labels Dec 30, 2020
@PazerOP PazerOP changed the title [Enhancment] Backup playerlist in event of malformed json Backup playerlist in event of malformed json Dec 30, 2020
@PazerOP PazerOP added this to the 1.2 milestone Dec 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Priority: Medium This issue may be useful, and needs some attention. Type: Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants