Handle malformed JSON request body

[nvlled]

• fixes "local variable 'params' referenced before assignment" when JSON is malformed
• return "unsupported action" when JSON body is not an object
• refactor to make if-statements more readable

[oakkitten]

Hello, I wanted to tackle this problem as well. I did it a bit differently.

• Probably it doesn't really matters, but since I had to chose one of two ways, I chose not to return the error information if the request is not allowed the origin check. It's technically safer I guess? Not sure if it make a difference in a real world.
• And, I opted to return the actual JSON error, if allowed, since it's useful.

This method would greatly benefit from a complete rewrite, but I didn't want to make a big change so I opted for the smallest change. Here's the diff if you are curious (I didn't write tests at the time): https://github.com/oakkitten/anki-connect/commit/954dae6327c032f18a54b708f6463b203029fd94.

[nvlled]

Hey, I didn't know this is already being worked on. I'm not really that experienced in python anyway, so let's go with yours.

I chose not to return the error information if the request is not allowed the origin check. It's technically safer I guess?

I'm not a security expert by any means, but I do I agree limiting the information from unauthorized hosts is indeed better. I think I failed to account for that one.

And, I opted to return the actual JSON error, if allowed, since it's useful.

This may leak some implementation details that may not be necessarily useful for API consumers, especially for non-python users. A custom error message might be better.

[oakkitten]

Oh I didn't mean to imply that this issue is mine or anything, I did touch it but never told anyone or even push it to my fork before I saw this PR. Just wanted to put in my two cents so to say.

This may leak some implementation details that may not be necessarily useful for API consumers

Perhaps, but then Anki-Connect normally does output exception strings. And, end users most likely will never see this message, for whatever they use will probably use a cute and neat JSON library that never outputs bad JSON. And if they are constructing the JSON themselves, they are likely to enjoy the actual message even if they don't know Python, eh?

[nvlled]

I don't think I've ever seen a JSON library that doesn't barf out an exception with the slightest error, even with trailing commas. But that aside, speaking from my recent experience, I do tend to construct JSON manually when exploring the API with curl. Suppose I was a new user to the API, and I see very specific messages like Extra data: line 1 column 3 (char 2) or Expecting value: line 1 column 1 (char 0), it's not immediately obvious what extra data or extra value means here. I might mistakenly believe that this is a bug in the API. A brief message like Syntax error, Parse error, or even JSON error would suffice.

[oakkitten]

I meant that if the user is using something like Yomichan, under the hood it is constructing a very proper JSON and there's no way they would see the cryptic error message. And if the user is constructing JSON themselves, they probably won't get confused by the exact error.

Still, you have a point. There's still a case where a user might use some poor software that constructs JSON in a manual way. And, something like Expecting value: line 1 column 1 (char 0) can be confusing even if you are a seasoned developer since this doesn't even say that it is a JSON error. I think the proper solution here would be introduce a new API version and send the entire traceback as the error message, something like

JSONDecodeError: Expecting property name enclosed in double quotes: line 1 column 10 (char 9)

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.9/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "/usr/lib/python3.9/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib/python3.9/json/decoder.py", line 353, in raw_decode
    obj, end = self.scan_once(s, idx)
json.decoder.JSONDecodeError: Expecting property name enclosed in double quotes: line 1 column 10 (char 9)

This would also solve the issue of Anki raising errors without text, so the user gets {result: null, error: ""}—not helpful!