Skip to content

Large array is parsed as an object #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rusuly opened this issue Jan 31, 2020 · 6 comments
Closed

Large array is parsed as an object #59

rusuly opened this issue Jan 31, 2020 · 6 comments
Assignees

Comments

@rusuly
Copy link
Contributor

rusuly commented Jan 31, 2020

$this->parseObject(BinaryDataReader::UNSIGNED_INT32_LENGTH);

Hi, again.
I think I spotted a bug when looking at the class.
Since mysql-binlog-connector-java hasn't been supported for a while I was looking for how different libraries handle empty holes made by partial updates in JSON(some complain it occurs in MySQL 8.0.16+).

@krowinski
Copy link
Owner

Hi, good catch its should be array. Funny thing is that in php you can decide when unserialising json is array|object so event if I decode as object still you can do json_encode($json, true) and get array...

Fix and test should be soon tx!

@krowinski krowinski self-assigned this Feb 3, 2020
@rusuly
Copy link
Contributor Author

rusuly commented Feb 3, 2020

Also, be informed that in-place updates make empty holes between keys and values as described in the spec

Steps to reproduce (redproduced from MySQL 8.0.16+):

`
UPDATE t1 SET JsonColumn='{"id": 1, "name": "Monica", "types":[1,"123"]}' WHERE Id = 1;
UPDATE t1 SET JsonColumn = JSON_SET(JsonColumn, '$.name', 'Mon') WHERE id = 1;
`

This will create 3-byte hole in the "name" value so you need to advance the stream.

@krowinski
Copy link
Owner

krowinski commented Feb 11, 2020

hmm even is you do

UPDATE t1 SET j='{"id": 1, "name": "Monica", "types":"test"}' WHERE id = 1
UPDATE t1 SET j = JSON_SET(j, '$.name', 'Mon') WHERE id = 1

it will work but you get strange json

"{"id":1,"name":"Mon","types":"catest"}"

its take from Monica - ca and add to types 🙄

raw binary data

string(63) "i�B^���P��10$+idnametypesMonicatest10$+idnametypesMonicatestb�y" hmm

@krowinski
Copy link
Owner

krowinski commented Feb 12, 2020

I don't see any other way to skip unused offset like it was done here

johnyang007/mysql-binlog-connector-java@2fdea43

furthermore I don't see any solution even in mysql-server code

https://github.com/mysql/mysql-server/blob/8.0/sql/json_binary.cc

  the length byte with the new length, and the beginning of the original string
  data. Since the original string "abc" is longer than the new string "XY",
  we'll have a free byte at the end of the string. This byte is left as is
  ('c'). 

Its should work as we read binary data and still it's not cutting correctly.

@rusuly
Copy link
Contributor Author

rusuly commented Feb 12, 2020

Hi,

In my lib I calculate unused space and skip it using Advance method

I think we don't see it in mysql-server code because it is written in C and it accesses memory directly using offsets(it doesn't read the stream in cursor-like style like our libraries do).

@krowinski
Copy link
Owner

Ok its fixed now, added new test based on yangkun code to verified this and looks good. Also I redone all code based on mysql server code.

@rusuly rusuly closed this as completed Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants