core: services: bag of holding: protect against bad API calls

[ES-Alexander]

Current behaviour

It's possible to break the bag by putting in a poorly formed set API call, e.g. by having a trailing slash on the path (e.g. .../bag/v1.0/set/path/), which results in things like

  "path": {
    "": {
      "variable": 9
    }
  }

(and causes failed subsequent get requests), instead of the desired .../bag/v1.0/set/path ->

  "path": {
    "variable": 9
  }

Expected or desired behaviour

1. Where possible, bag should protect itself from API calls that can break its usage for subsequent calls
2. If practical bag should try to fix broken calls into something more functional (e.g. by stripping trailing slashes), or at least reject them so they don't break it for the other services

Prerequisites

• [X] I have checked to make sure that a similar request has not already been filed or fixed.

[rafaellehmkuhl]

I have reached the same problem.

In one of my Cockpit syncs, somewhat happened and the bag-of-holdings json file got broke, which caused the service to die till I stopped it and deleted it's cache.

broke_bag.json

This seems like a critical problem to me. Any service relying in the bag of holdings can and will be affected if anything goes wrong, and there's no user-friendly way to fix it.

[ES-Alexander]

Your posted json seems to be cut off early, so perhaps failed partway through writing or something?

As discussed in the meeting, it would be nice if the service keeps a temporary "always valid" backup file of the current state during request processing, and checks whether each attempted change to the state fails to be read back after saving, in which case it can roll back to the backup and reject the request.

If it restarts while writing or something then when it attempts to start up again it can load the backup file if it fails to load the primary file.

This seems like a critical problem to me.

I definitely agree - several other services rely on the bag, so having simple ways to break it is problematic.