GMC BCP parser breaks on "?" character in settings strings

[bosh]

I've been running into weird behavior and a json message sent to the gmc bcp parser that causes it to hurl, but I've figured out how to avoid the issue for now. TLDR is I had a '?' in a settings item label which caused an unexpected line split in bcp parser.I was seeing the error:

E 0:00:07:0938   bcp_parse.gd:34 @ string_to_obj(): Parse JSON failed. Error at line 0: Unterminated String
  <C++ Error>    Condition "error != Error::OK" is true. Returning: Variant()
  <C++ Source>   core/io/json.cpp:573 @ parse_string()
  <Stack Trace>  bcp_parse.gd:34 @ string_to_obj()
                 bcp_parse.gd:15 @ parse()
                 bcp_server.gd:299 @ _thread_poll()

due to truncated settings json (variable "message').The split on "?" on bcp_parse:13 resulted in split_message[1] being the invalid json body:

json={"settings": [["brightness", "Brightness", 100, "brightness", 1.0, {"0.25": "25%", "0.5": "50%", "0.75": "75%", "1.0": "100% (default)"}, "standard"], ["ball_search_enabled", "Ball search enabled

My settings.yaml is defined as:

#config_version=6

settings:
  ball_search_enabled:
    sort: 200
    label: Ball search enabled?
    values:
      0: "Disabled"
      1: "Enabled"
    default: 0
    key_type: int

Tracing back up I found that split_message[2] exists with value:

", 200, "ball_search_enabled", 0, {"0": "Disabled", "1": "Enabled"}, "standard"], ["flipper_power", "Flipper power", 1000, "flipper_power", 1.0, {"0.8": "Weak", "1.0": "Normal (default)", "1.2": "Strong"}, "standard"]]}

So it looks like ? in label text leads to no fun.It seems reasonable that a user might use a ? in one of these strings, but for now I'm just dropping the character from my text. I'm happy to implement a fix or add some robustness, if we do want to support the ?.

Options seem to be:

• rejoin split_message[1..] on ? before json parsing
• encode message body to prevent any special character issues and decode (seems p heavyweight imo)
• discourage ? use and add note to docs (or add full on text validation warnings/errors)

Thoughts?

[bernarma]

I'm not sure but if you use quotes or represent that character in a different encoding work?

[avanwinkle]

Good find! I wasn't expecting any question marks in strings, so the separation of the BCP message was minimal. I've added a change to restrict the message parsing to only one question mark, so any question marks in the JSON body will be part of their strings.

This isn't a perfect solution if there were to be a non-JSON payload that had a question mark, but I can't think of a scenario where that would be the case so I think it should be fine for now.

This fix is available in the latest main branch.

[bosh]

Just verified the fix removes the crash. Thanks!

[bosh]

Oh I just realized this should have been an issue in that repo, not mpf main. My bad :)