wp-net / WordPressPCL

This is a portable library for consuimg the WordPress REST-API in (almost) any C# application
MIT License
335 stars 131 forks source link

The library can throw Newtonsoft.Json.JsonReaderException in some cases when WP_DEBUG_DISPLAY is true in wordpress configuration #273

Closed navjot50 closed 2 years ago

navjot50 commented 2 years ago

I tried to upload an image using WordpressPCL and I got a Newtonsoft.Json.JsonReaderException with a message "Unexpected character encountered while parsing value: <. Path '', line 0, position 0.". I found out that the image was successfully uploaded on the wordpress server but not assigned to MediaItem item = await client.Media.Create(imageStream, "img.jpg");, instead there was the above exception.

Further investigating, I found out that the json response from the wordpress server was preceded by PHP warnings markup. A cut down version of the response looked like:

<br />
<font size='1'><table class='xdebug-error xe-warning' dir='ltr' border='1' cellspacing='0' cellpadding='1'>
<tr><th align='left' bgcolor='#f57900' colspan="5"><span style='background-color: #cc0000; color: #fce94f; font-size: x-large;'>( ! )</span> Warning: exif_read_data(img.jpg): Incorrect APP1 Exif Identifier Code in C:\Users\Scintec AG\Sites\www.scintec.dev.cc\wp-admin\includes\image.php on line <i>801</i></th></tr>
<tr><th align='left' bgcolor='#e9b96e' colspan='5'>Call Stack</th>
//followed by some more markup here
{"id":2171,"date":"2021-10-06T13:59:55","date_gmt":"2021-10-06T13:59:55" ...} //json to be serialized by library

So, basically the library was expecting no markup in front of json. I found out that this markup comes up only when WP_DEBUG_DISPLAY is set to true. In development phase this constant is set to true most of the times. So, the library should handle this because the image is uploaded on the server but the json for it could not be deserialized by the library.

I can give a PR to fix this issue.

ThomasPe commented 2 years ago

interesting, I agree this could be handled by the library. What would be your approach?

navjot50 commented 2 years ago

My approach would be:

  1. Check if the response starts with Json opening bracket (in case of warnings it will have the standard html table for php warnings and errors)
  2. If the Json opening bracket is not the first character, then I will search via a regex like {\"id\":.+}. I think this regex is enough and a more strict recursive regex for json syntax is not required because wordpress rest api responses are supposed to start with id as the first property in json.
  3. If nothing matches to regex, that means there is no json to be deserialized. So, I will wrap the response in WPUnexpectedException.
  4. If a match exists then only that needs to be deserialized and returned.
  5. Also, for more reusable usage, the Json functionalities can be shifted to a helper class and the JsonConvert.DeserializeObject is being used in HttpHelper and ThreadedCommentsHelper.

The regex will come into play only when the response has markup pollution.

ThomasPe commented 2 years ago

sounds like a plan to me. are there instances where an array is returned from the API?

navjot50 commented 2 years ago

Correct. Array is returned in case of multiple entries. So, that case also needs to be taken care of and in that case the response starts with [.

ThomasPe commented 2 years ago

Merged and part of the first 2.0-alpha release