search

halaxa / json-machine

Efficient, easy-to-use, and fast PHP JSON stream parser
Apache License 2.0
1.1k stars 65 forks source link

Getting un catchable errors on non JSON files #86

Closed flavinus closed 2 years ago

flavinus commented 2 years ago

When calling fromFile on a wrong file format I get some uncatchable errors. I had this issue with both 0.7.0 and 1.1.1

\JsonMachine\JsonMachine::fromFile($filename, "/myattribute");

I get the following fatal error with a zip file, but I can also have similar issues with text files

message: Undefined variable $P script: .../halaxa/json-machine/src/Parser.php line: 115

Testing vars before $tokenType = ${$token[0]}; seems to be helful.

if($token == null || !isset($token[0])|| !isset(${$token[0]})) { throw new JsonMachineException("Error parsing stream."); }

halaxa commented 2 years ago

If I get it right, do you mean when you feed JSON Machine a compressed zip file? That's usually not what you want to do in the first place, right? :)

flavinus commented 2 years ago

No, this is not what I want in my application but this can happen because of user input file. Of course I will verify the file type before trying to parse it, but it could be interesting to improve robustness of your library (thanks it is very usefull :) )

halaxa commented 2 years ago

I was just curious. You're right of course. The reason for this error is that the library is heavily optimized for performance thus ignoring some edge-case lexical errors, which a json token starting with capital P is. Those produce not very meaningful errors, such as yours. I'll look into handling such errors without hitting the performance.

halaxa commented 2 years ago

I'm failing to reproduce the error. Can you post the data or attach a failing test with this error? Place it in ParserTest please.

halaxa commented 2 years ago

Looking at it closer, the line $tokenType = ${$token[0]}; you mention is not there since version 1.0.1. It was removed here eeb2051bec8b7024d392ab28337ed7d2a68d95d8. That's why I haven't been able to replicate it in 1.1.1. The problem still persists as the Undefined array key error though. I'll look into it.

halaxa commented 2 years ago

Fixed on master. Check it out.

halaxa commented 2 years ago

Released in 1.1.2

flavinus commented 2 years ago

Thank you very much!!! Sorry I was too busy and I didn't took time to help you for rproduction.

halaxa commented 2 years ago

No problem. I decided to early-release after the comment. I hope the unit test is enough. 😁 However feel free to reach out if you have any problems.