Raise ValidationError when other type than json is provided during Ad… #1850
Conversation
|
Attached issue: https://pulp.plan.io/issues/7468 |
pulp_rpm/app/serializers/advisory.py
Outdated
| try: | ||
| update_record_data.update(json.loads(data["file"].read())) | ||
| update_record_data.update(data) | ||
| except ValueError as e: |
There was a problem hiding this comment.
A RPM file rose UnicodeDecodeError at data["file"].read()
testing a format different than JSON I got:
In [1]: import json
In [2]: json.loads("a")
---------------------------------------------------------------------------
JSONDecodeError Traceback (most recent call last)
<ipython-input-2-c0c0ca6f67c6> in <module>
----> 1 json.loads("a")
~/.pyenv/versions/3.8.0/lib/python3.8/json/__init__.py in loads(s, cls, object_hook, parse_float, parse_int, parse_constant, object_pairs_hook, **kw)
355 parse_int is None and parse_float is None and
356 parse_constant is None and object_pairs_hook is None and not kw):
--> 357 return _default_decoder.decode(s)
358 if cls is None:
359 cls = JSONDecoder
~/.pyenv/versions/3.8.0/lib/python3.8/json/decoder.py in decode(self, s, _w)
335
336 """
--> 337 obj, end = self.raw_decode(s, idx=_w(s, 0).end())
338 end = _w(s, end).end()
339 if end != len(s):
~/.pyenv/versions/3.8.0/lib/python3.8/json/decoder.py in raw_decode(self, s, idx)
353 obj, end = self.scan_once(s, idx)
354 except StopIteration as err:
--> 355 raise JSONDecodeError("Expecting value", s, err.value) from None
356 return obj, end
JSONDecodeError: Expecting value: line 1 column 1 (char 0)There was a problem hiding this comment.
https://docs.python.org/3/library/json.html#exceptions
In [12]: from json import JSONDecodeError
In [13]: try:
...: json.loads('/home/vagrant/devel/pulp_rpm/docs/_scripts/bear-4.1-1.noarch.rpm')
...: except JSONDecodeError:
...: print("lala")
...:
...:
...:
...:
lala
In [14]: try:
...: json.loads('/home/vagrant/devel/pulp_rpm/docs/_scripts/bear-4.1-1.noarch.rpm')
...: except ValueError:
...: print("ina")
...:
...:
ina
There was a problem hiding this comment.
oh! it is a subclass!
today I learned!
thank you @ipanova
There was a problem hiding this comment.
While JSONDecodeError is a subclass of ValueError, I think it would be "cleaner" to catch the specific exception. Otherwise a ValueError coming from some different location could be hidden. Granted I don't see such a location in the current code, I think it's a good idea to be as specific as possible.
|
Could you add a test please? |
pulp_rpm/app/serializers/advisory.py
Outdated
| update_record_data.update(data) | ||
| try: | ||
| update_record_data.update(json.loads(data["file"].read())) | ||
| update_record_data.update(data) |
There was a problem hiding this comment.
Does this line needs to be in the try/except clause?
There was a problem hiding this comment.
normally i would move this to the else clause however in this case this line does not pollute much, imo.
pulp_rpm/app/serializers/advisory.py
Outdated
| update_record_data.update(json.loads(data["file"].read())) | ||
| except json.JSONDecodeError: | ||
| raise serializers.ValidationError( | ||
| "Json file is expected" |
CHANGES/7468.bugfix
Outdated
| @@ -0,0 +1 @@ | |||
| Raise ValidationError when other type than json is provided during Advisory upload. | |||
| update_record_data.update(json.loads(data["file"].read())) | ||
| update_record_data.update(data) | ||
| try: | ||
| update_record_data.update(json.loads(data["file"].read())) |
There was a problem hiding this comment.
😆 🤣 That's a solution to address PR comments ;)
ipanova
force-pushed
the
i7468
branch
4 times, most recently
from
68430b0
to
7c2f97c
Compare
…visory upload. [nocoverage] closes #7468 https://pulp.plan.io/issues/7468
…visory upload.
closes #7468
https://pulp.plan.io/issues/7468