Repository navigation
HTML entities in responses not decoded for some endpoints (raw_json=1 not passed) #148
Description
This issue is related to #83. I'm using version 1.5.0.
It looks like most of the Reddit API calls from this library include raw_json=1 to disable the HTML entity decoding mentioned in Reddit's API documentation.
Unfortunately, some API calls appear to not include raw_json=1, so the responses from Reddit can have encoded HTML entities. Specifically, I ran into this when calling RedditClient.GetPosts:
var reddit = new RedditClient(accessToken: "...");
var posts = reddit.GetPosts(new List<string> { "t3_ty368b" });
Console.WriteLine(posts[0].Title);
// actual: Ocean Gray & Silver No. 2/65 Keycult
// expected: Ocean Gray & Silver No. 2/65 Keycultvar reddit = new RedditClient(accessToken: "...");
var posts = reddit.GetPosts(new List<string> { "t3_tvrcgx" });
Console.WriteLine(posts[0].Listing.URL);
// actual: https://twitter.com/TarjeiJS/status/1510724911001747459?t=WrUlz-PyXBv_apofabipww&s=19
// expected: https://twitter.com/TarjeiJS/status/1510724911001747459?t=WrUlz-PyXBv_apofabipww&s=19It appears that RedditClient.GetPosts eventually calls into Listings.GetByNames:
public PostContainer GetByNames(string names)
{
return JsonConvert.DeserializeObject<PostContainer>(ExecuteRequest("by_id/" + names));
}Most of the other Listings methods use SendRequest and pass in an input model that ultimately derives from BaseInput which sets raw_json=1. If I change the GetByNames method to the following, everything appears to work as expected:
public PostContainer GetByNames(string names)
{
return SendRequest<PostContainer>("by_id/" + names, new BaseInput());
}From what I can tell, it looks like this might affect multiple API calls from this library. It seems that all API calls need to be using SendRequest with some model deriving from BaseInput or ExecuteRequest/PrepareRequest should be adding raw_json=1 to all requests.
Activity
sirkris commentedon Jun 27, 2022
Yeah this behavior was intentional. In the API docs, raw_json
iswas only mentioned as a valid parameter for most endpoints, but therearewere several thatdon'tdidn't include it. When I wrote the library, I chose to take this literally and interpreted it to mean that this parameter should only be present on those endpoints where it's documented. It's also possible that this was merely an unintentional omission on the part of the Reddit devs but I had to make a choice.So just FYI, the fact that they inherit from different classes that omit raw_json is by design. I should also note that this architecture has not resulted in any test failures. That said, I don't think any of the tests check for HTML entities, so that might explain it.
To be clear, are you saying that ALL endpoints should be passing raw_json=1? Do we know for certain that this won't cause any to break? I'm really nervous about making any changes that contradict the API docs, so I want to be absolutely sure this won't end up causing some endpoints to break down the road.
sirkris commentedon Jun 27, 2022
Ok so I went back to the API docs for the first time in some years and found this section:
Seems they've improved that since I wrote this library 4 years ago. Back then, "raw_json" was listed in the parameters list for most (but not all) endpoints, leading me to believe it only applied to those endpoints. I don't remember ever seeing that "response body encoding" section back then so I'm guessing that's probably new. That makes sense considering raw_json is no longer listed in the parameters list for any of the endpoints.
Now that I have confirmation that it's not endpoint-specific, I'll update accordingly.
sirkris commentedon Jun 27, 2022
Fixed and merged into develop. Also applied the change to Listings.Random().
schmich commentedon Jun 27, 2022
Fantastic, thank you for your help! I wouldn't be surprised if this was just a documentation issue they had.
I know maintaining a client library can be tough for such a large API, so thank you again for what you've built here. If you have a ko-fi/buymeacoffee/etc. account, let me know.