Skip to content

[bitreq] Check utf-8 while deserializing JSON body #486

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

Merged
tnull merged 2 commits into from
Jan 30, 2026
Merged

[bitreq] Check utf-8 while deserializing JSON body #486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f12c062
[bitreq] Check utf-8 while parsing JSON responses
tankyleo Jan 28, 2026
2c3bbf2
[bitreq] Serialize `Request` body into `Vec<u8>`
tankyleo Jan 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bitreq/src/request.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ impl Request {
pub fn with_json<T: serde::ser::Serialize>(mut self, body: &T) -> Result<Request, Error> {
self.headers
.insert("Content-Type".to_string(), "application/json; charset=UTF-8".to_string());
match serde_json::to_string(&body) {
match serde_json::to_vec(&body) {
Ok(json) => Ok(self.with_body(json)),
Err(err) => Err(Error::SerdeJsonError(err)),
}
Expand Down
2 changes: 1 addition & 1 deletion bitreq/src/response.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ impl Response {
where
T: serde::de::Deserialize<'a>,
{
match serde_json::from_str(self.as_str()?) {
match serde_json::from_slice(self.as_bytes()) {
Copy link
Member

@TheBlueMatt TheBlueMatt Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really prefer the serde error over the utf8 error? I'm skeptical the claimed perf difference matters (notably when deserializing a string in the json serde skips utf8 validation if you call from_str whereas has to do it if you call from_slice). I don't have a strong feeling on the error but the commit should explain why we prefer the swap.

Copy link
Contributor Author

@tankyleo tankyleo Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Matt see below, I have expanded the commit message.

I'm skeptical the claimed perf difference matters

The claim is that a reduction in cache misses leads to non-trivial performance gains.

We still do the same amount of work on the data.

Ok(json) => Ok(json),
Err(err) => Err(Error::SerdeJsonError(err)),
}
Expand Down