Produce a better error in case of malformed JSON

[oakkitten]

This solves the same issue that #317 solves, albeit slightly differently. I was hoping that the author would reopen the issue... There's a few differences;

• If origin is not allowed, 403 is returned in case of malformed JSON, and
• In case of JSON error, error string is returned to user.

I also noticed this bit: params['params'] = params.get('params', {}). It's a tad dangerous since this part of code can be run when not allowed, and params can be present but not be a dictionary. In this case the plugin would “crash”, and while this only amounts to showing a window with an error, it can be theoretically triggered by a malicious website, so I opted for verifying request schema. (Anki comes with jsonschema which is used to verify addon configurations.) This is not ideal, but I wanted to touch as little code as possible.

I added a few tests that test behavior around the change.

@nvlled Could you please look at this and tell if I did something stupid? 😬

[FooSoft]

Looks great to me :)

[oakkitten]

I really wish @nvlled said something, it now feels like I stole their PR :<

[nvlled]

Sorry, I couldn't find the time to give a thorough review. But I do one nit: explicity initialize params outside of the try block. Something like:

params = {}
try:
    params = json.loads(req.body.decode('utf-8'))

The bug I encountered was specifically caused by an uninitialized params variable. It would be nice to prevent this from happening when changes are made in the future.

[oakkitten]

Ah, that should be solved in this PR! In the except part of the of the block, if continuing, the params variable is set. I suppose one could also set it before like in your example. It's just that I use PyCharm and I'm accustomed to it warning me if I forget to set a variable in one of the execution branches.

anki-connect/plugin/web.py

Lines 179 to 192 in 9c3310a

	try:
	params = json.loads(req.body.decode('utf-8'))
	jsonschema.validate(params, request_schema)
	except (ValueError, jsonschema.ValidationError) as e:
	if allowed:
	if len(req.body) == 0:
	body = f"AnkiConnect v.{util.setting('apiVersion')}".encode()
	else:
	reply = format_exception_reply(util.setting('apiVersion'), e)
	body = json.dumps(reply).encode('utf-8')
	headers = self.buildHeaders(corsOrigin, body)
	return self.buildResponse(headers, body)
	else:
	params = {} # trigger the 403 response below

I'd still appreciate the review! Anyway, I guess I better make this not a draft since, well, the button says “Ready for review” :p

[FooSoft]

Looks good to me!

[oakkitten]

P.S. the tests are failing now due to protobuf update (packages anki/aqt don't pin dependencies), I'll have a fix shortly

[FooSoft]

Thanks!