Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix parsing from QIODevice #98

Closed
wants to merge 8 commits into from
Closed

Fix parsing from QIODevice #98

wants to merge 8 commits into from

Conversation

gooseensky
Copy link
Contributor

[] Move parsing to private class;
[] Use same entry point for both parse() API;

@gooseensky
Fix parsing from QIODevice
940a1d9 
[*] Move parsing to private class;
[*] Use same entry point for both parse() API;
@gooseensky
Copy link
Contributor Author

gooseensky commented Jan 19, 2017 •
edited

When reading from QNetworkReply all data will be read at once.
It breaks internal logic, please check the following places:
https://github.com/flavio/qjson/blob/master/src/json_scanner.cc#L3214,
https://github.com/flavio/qjson/blob/master/src/json_scanner.cpp#L74,
https://github.com/flavio/qjson/blob/master/src/json_scanner.cc#L3983

Will fix #52

Some logs for fail case:
23:28:25.686 DEBUG OAuthenticator:113 "code=6336001&client_id=36f9cfadbf8c4f87ae9fbead439a8a6d&client_secret=afb61b309d144fdab1c49fd2014c60b1&grant_type=authorization_code&redirect_uri=https%3A%2F%2Fclementine-data.appspot.com%2Foauth"
23:28:25.808 DEBUG unknown Data available before read: 222
23:28:25.808 DEBUG unknown Bytes read: 222
23:28:25.808 DEBUG unknown Data available after read: 0
23:28:25.808 DEBUG unknown Data available before read: 0
23:28:25.808 ERROR unknown JSonScanner::yylex - error while reading from io device
23:28:25.808 ERROR unknown json_parser - syntax error found, forcing abort, Line 1 Column 1

drizt and others added 5 commits July 14, 2017 15:41
@drizt
Fix static linking on Windows
141da7d 
CMake can mistakenly think that installed Qt4 is shared and add
-DQT_DLL to QT_DEFINITIONS. Need to be sure that with static QJson
will be used static Qt4.
@flavio
Merge pull request flavio#100 from drizt/win-static
eecad30 
Fix static linking on Windows
@flavio
Update README to include contributio guidelines
7760268
@flavio
Add travis support
a998881 
Support building with Qt4 and Qt5
@flavio
Add travis badge
e78cf9e
@flavio
Copy link
Owner

flavio commented Jul 15, 2017

I just added travis CI support, could you rebase this PR against master to enable travis runs on this PR?

@gooseensky
Fix parsing from QIODevice
fb5d732 
[*] Move parsing to private class;
[*] Use same entry point for both parse() API;
@gooseensky
Merge branch 'issue#52' of https://github.com/selumin/qjson into issu…
f73e5bf 
…e#52
@flavio
Copy link
Owner

flavio commented Jul 20, 2017

I manually cherry-picked your fixed into master, you did a merge instead of a rebase. That would have polluted the history of the repo ;)

@flavio flavio closed this Jul 20, 2017
@gooseensky
Copy link
Contributor Author

Sorry for this.
And thanks for help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@gooseensky @flavio @drizt