TheHive4py icon indicating copy to clipboard operation
TheHive4py copied to clipboard

JSON decode error fix

Open wizedkyle opened this issue 1 year ago • 3 comments

Outlined in #255 there can be JSON decode errors when running TheHive4Py on AWS Lambda most likely due to Lambda not including the correct package that contains JSONDecodeError.

This PR updates the JSONDecodeError to come from the requests package. Thank you to all the hard work from @octocolby for identifying the root cause of this issue and providing a fix.

wizedkyle avatar Oct 04 '22 07:10 wizedkyle

I've made a fix for this one before, but it seems like the PR got lost in the meantime.

Did you try this fix against simplejson?

In case it doesn't work with other libs please fall back using this snippet:

import json as jsonlib

And inside the try block use the standard lib to decode JSON instead of the builtin json() method of requests.

json_data = jsonlib.loads(response.content)

Kamforka avatar Oct 04 '22 08:10 Kamforka

@Kamforka No we didn't try against simplejson however, requests will decide if there is simplejson or json so wouldn't that be ideal as you don't have to worry about the lib it will determine it for you?

I will add my Python knowledge isn't as good as the people here so i will also defer to @octocolby if he has any other thoughts.

https://github.com/psf/requests/blob/bda7f0171f8bba17989d3a2c28dfa9a9261b1b65/requests/compat.py#L39

wizedkyle avatar Oct 04 '22 08:10 wizedkyle

@wizedkyle I'm not sure requests will choose the right decode error as there is a thread around this issue for years now: https://github.com/psf/requests/issues/4842

I don't mind your current solution if you can verify it against simplejson, I'd do that myself but for the time being I don't have access to my workspace, I can check it earliest end of this week.

Kamforka avatar Oct 04 '22 12:10 Kamforka

@Kamforka I am going to test the fix against simplejson tonight and will let you know the results.

wizedkyle avatar Oct 10 '22 06:10 wizedkyle

@Kamforka using simplejson fixes the issue as well, so which fix do you want to use?

wizedkyle avatar Oct 10 '22 07:10 wizedkyle

@wizedkyle Maybe I explained myself poorly. I didn't mean to use the simplejson inside thehive4py, what I meant is that your original fix should be tested against an environment where simplejson is present, i.e. pip install simplejson thehive4py in your testing environment.

But no worries now I also have to verify it on my own, I'll get back to you.

Kamforka avatar Oct 19 '22 11:10 Kamforka

Okay I've checked the requests way of doing things and I'm not sure why it worked for you but I immediately get this error:

>>> from requests import JSONDecodeError
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: cannot import name 'JSONDecodeError' from 'requests'

So I'd just suggest to fall back to my original recommendation and use the standard json library to parse the content of the response:

json_data = jsonlib.loads(response.content)

May I ask you to add this change to the PR? You need to adjust _process_text_response and _process_error_response for this.

I also tested it in an environment with and without simplejson and it worked fine.

Kamforka avatar Oct 19 '22 11:10 Kamforka

Fixing this in #259

Kamforka avatar Oct 21 '22 07:10 Kamforka