JSONDecoder.raw_decode breaks on leading whitespace

[aalien]

BPO	15393
Nosy	@jcea, @rbtcollins, @ezio-melotti, @bitdancer, @squidsoup, @Mariatta, @DanielNoord
Files	json-raw-decoder-fix-whitespace.diff: Patch for Python 3.3.0 json-raw-decoder-fix-whitespace.diff: Patch for Python 3.3.0, take two issue15393_3.6.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2012-07-19.10:09:55.376>
labels = ['3.7', 'easy', 'type-bug', 'library']
title = 'JSONDecoder.raw_decode breaks on leading whitespace'
updated_at = <Date 2022-03-18.20:05:49.519>
user = 'https://bugs.python.org/aalien'

bugs.python.org fields:

activity = <Date 2022-03-18.20:05:49.519>
actor = 'danielnoord'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2012-07-19.10:09:55.376>
creator = 'aalien'
dependencies = []
files = ['26434', '26450', '44613']
hgrepos = []
issue_num = 15393
keywords = ['patch', 'easy']
message_count = 14.0
messages = ['165829', '165830', '165831', '165832', '165834', '165835', '165836', '165918', '168936', '168939', '276143', '276161', '276291', '415523']
nosy_count = 8.0
nosy_names = ['jcea', 'rbcollins', 'ezio.melotti', 'r.david.murray', 'aalien', 'Bayard Randel', 'Mariatta', 'danielnoord']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue15393'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

Linked PRs

• gh-59598: Ignore leading whitespace in JSONDecoder.raw_decode #117397

[aalien]

raw_decode on json.JSONDecoder does not handle leading whitespace. According to RFC 4627, section 2, whitespace can precede an object. With json.loads leading whitespace is handled just fine.

d = json.JSONDecoder()
d.raw_decode(' {}')
ValueError: No JSON object could be decoded

Tested with versions 2.6.7, 2.7.3, 3.1.3 and 3.2.3

[aalien]

My coworker just submitted a pull request for a possible fix to simplejson on github.

simplejson/simplejson#38

[jcea]

Please, post a patch for 2.7 and 3.2/3.3, with a test. Seems quite easy.

If you hurry, this could go in 3.3.0.

[aalien]

Suggestion for a patch for 3.3.0. I wasn't quite sure how the testcases were supposed to be loaded. Sorry if I made a mess ;)

[jcea]

Antti, you are changing the signature of "decode()" and that would be a compatibility problem. Can you rewrite the patch to be more "compatible"?

In your test, please, use a bit more complicated object than "{}" :-)

[bitdancer]

IMO this is not a bug, according to the current documentation it is working as designed. raw_decode says it decodes a string that *starts with* a json document.

To my understanding, raw_decode is designed to be used when parsing a stream containing more than just one json document, including possibly non-json text, in which case it makes sense that the calling application would be responsible for (and want to) handle the whitespace around any such document.

I recommend closing this as invalid. If the consensus is otherwise and a change is made, I think it would be an enhancement, not a bug fix.

[bitdancer]

Ah, I see, you are thinking that "json document" includes the possibility of leading whitespace. That is a reasonable interpretation...just make sure that we don't break backward compatibility.

[aalien]

you are changing the signature of "decode()" and that would be a compatibility problem

I was changing the signature of raw_decode(), by adding a(n almost) private keyword. I really don't see how that would affect compatibility.

you are thinking that "json document" includes the possibility of leading whitespace

Yes. While JSON does not have a real standard to follow, I believe that going by the RFC is the best thing to do, and the RFC very clearly states, that objects can be surrounded with whitespace.

decode() depends on the functionality of raw_decode(), and its own parameter _w. I have no idea why _w is a parameter. It is not used anywhere in CPython, and, beginning with an underscore, it shouldn't be used from anywhere else. Still it offers the users a possibility to break the behaviour of decode(). If it's a performance hack, making the variable being initialized only once, then I think it's a very poor one.

Now the patch leaves decode() as it is, and checks for whitespace in raw_decode(), which leads to checking for whitespace twice when using decode().

I think that a more extensive cleanup would be in order.

[akheron]

I think skipping preceding whitespace in raw_decode() wouldn't hurt, but skipping following whitespace would be wrong.

David: Any examples of how this could break backwards compatibility?

[bitdancer]

I didn't have anything specific in mind, just making a general comment about the care that needs to be taken in crafting the fix.

[squidsoup]

I've provided an updated patch for 3.6. Additionally I've updated the docstring for decoder.raw_decode to explain that whitespace at the beginning of the document will be ignored. I believe the concerns around the backwards incompatible change to the function signature were addressed by aalien's second patch.

[rbtcollins]

I think the patch should either be rejected, or also handle trailing spaces: if we're taking the RFC definition of whitespace not being structural then we should also eat trailing space, which will change the check for extra data in decode to just checking end == s.len().

Obviously a test for the trailing case needs to be added as well.