bugGNU Octave - Bugs: bug #60688, jsondecode does not handle some...

 
 

bug #60688: jsondecode does not handle some arrays of structs correctly

Submitter:  Mike Miller <mtmiller>
Submitted:  Fri 28 May 2021 06:05:27 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  siko1056
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 18 Aug 2021 06:01:27 AM UTC, comment #8: 

Thank you again for your great contribution.  I pushed your patch slightly modified (proper commit message, formatting) here:

https://hg.savannah.gnu.org/hgweb/octave/rev/eb768fc5e6b7

Kai Torben Ohlhus <siko1056>
Group Member
Tue 17 Aug 2021 10:45:26 AM UTC, comment #7: 

Thanks, no mention as contributor needed.

I've cross-checked the test cases with Matlab, but good to get a second pair of eyes on it as I might have missed some corner cases...


Anonymous
Tue 17 Aug 2021 08:11:06 AM UTC, comment #6: 

Would you like to be credited for your contribution (name, email, entry in the manual list of contributors)?

Kai Torben Ohlhus <siko1056>
Group Member
Tue 17 Aug 2021 08:09:54 AM UTC, comment #5: 

Thanks for the contribution 🙂

I oversaw this report.  After checking the patch for Matlab compatibility, it is likely to be applied.

Kai Torben Ohlhus <siko1056>
Group Member
Tue 17 Aug 2021 07:39:14 AM UTC, comment #4: 

I've also run into this issue recently. Attached a patch that fixes it.

Essentially `decode_array_of_arrays` didn't handle the cases very well where the "array in the array turned out to actually be a struct array".
I've also added a test case for 2D struct arrays which would fail without the patch.


assert (jsondecode ('[[{"foo": 42}, {"foo": 1.23}], [{"foo": 12}, {"foo": "bar"}]]'), struct("foo", {42 1.23; 12 "bar"}));






(file #51792)

Anonymous
Fri 28 May 2021 10:53:10 PM UTC, comment #3: 

Not a regression, these have probably always failed, they are just overlooked corner cases in the way that empty objects and singleton dimensions are handled.

I just rewound to https://hg.savannah.gnu.org/hgweb/octave/rev/38e22065d9ec and the same errors occur there.

Mike Miller <mtmiller>
Group Member
Fri 28 May 2021 10:15:14 PM UTC, comment #2: 

Were these always broken or is this a regression occurring after


changeset:   29708:fbcc3a680826
user:        Rik <rik@octave.org>
date:        Wed May 26 07:51:34 2021 -0700
summary:     Improve performance (> 100X) of jsondecode (bug #60625).


Rik <rik5>
Group administrator
Fri 28 May 2021 06:12:20 PM UTC, comment #1: 

I've extracted the test cases, and fixed some whitespace issues, attached as json.tst to this bug report.

(file #51494)

Mike Miller <mtmiller>
Group Member
Fri 28 May 2021 06:05:27 PM UTC, original submission:  

Some of the test cases that I reported to the jsonstuff package were overlooked when these functions were added to Octave. The following JSON strings decode incorrectly or error in Octave:


jsondecode ('[{}]')
ans =

  0x0 struct array containing the fields:


jsondecode ('[[{}]]')
ans =
{
  [1,1] =

    0x0 struct array containing the fields:


}

jsondecode ('[[{"a": 1}]]')
error: octave_base_value::array_value(): wrong type argument 'scalar struct'


These should all decode to a scalar struct.

And these cases are all wrong, they should decode to struct arrays with dimensions 2×1, 1×2, and 1×1×2, respectively:


>> jsondecode ('[{}, {}]')
ans =

  0x0 struct array containing the fields:


>> jsondecode ('[[{}, {}]]')
ans =
{
  [1,1] =

    0x0 struct array containing the fields:


}

>> jsondecode ('[[[{}, {}]]]')
ans =
{
  [1,1] =
  {
    [1,1] =

      0x0 struct array containing the fields:


  }

}


You can find these test cases in https://github.com/apjanke/octave-jsonstuff.

Mike Miller <mtmiller>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #51792:  fix_60688.patch added by None (7KiB - application/octet-stream)
file #51494:  json.tst added by mtmiller (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by siko1056 (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-08-18 siko1056 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2021-08-17 siko1056 StatusConfirmed Patch Submitted
        Assigned toNone siko1056
    2021-08-17 None Attached File- Added fix_60688.patch, #51792
    2021-05-28 mtmiller StatusNeed Info Confirmed
    2021-05-28 rik5 StatusNone Need Info
    2021-05-28 mtmiller Attached File- Added json.tst, #51494
    2021-05-28 mtmiller Carbon-CopyRemoved 80942 -

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code