Bump serde from 1.0.133 to 1.0.136

[dependabot]

Bumps serde from 1.0.133 to 1.0.136.

Release notes

Sourced from serde's releases.

v1.0.136

• Improve default error message when Visitor fails to deserialize a u128 or i128 (#2167)

v1.0.135

• Update discord channels listed in readme

v1.0.134

• Improve error messages on deserializing NonZero integers from a 0 value (#2158)

Commits

• 02bd79a Release 1.0.136
• c3ce2c9 Merge pull request #2167 from serde-rs/error128
• 0d71ac8 Render 128-bit integer value into Visitor errors
• 82c3eb7 Add test of visitor error messages
• 8932c85 Release 1.0.135
• 9f3dd3c Merge pull request #2163 from serde-rs/discord
• dd9b415 Add discord invite links
• 3bb4a5a Release 1.0.134
• 6164627 Merge pull request #2159 from serde-rs/nonzero
• 51aaf49 Write better Visitor for NonZero integers
• Additional commits viewable in compare view

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

---

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

• @dependabot rebase will rebase this PR
• @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
• @dependabot merge will merge this PR after your CI passes on it
• @dependabot squash and merge will squash and merge this PR after your CI passes on it
• @dependabot cancel merge will cancel a previously requested merge and block automerging
• @dependabot reopen will reopen this PR if it is closed
• @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
• @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
• @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
• @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

[codecov]

Codecov Report

Merging #3402 (9095712) into main (ac662df) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3402      +/-   ##
==========================================
+ Coverage   80.10%   80.11%   +0.01%     
==========================================
  Files         290      290              
  Lines       32841    32841              
==========================================
+ Hits        26306    26312       +6     
+ Misses       6535     6529       -6     

[dconnolly]

This introduces a new Buf type that has an as_str method that uses unsafe:

https://github.com/serde-rs/serde/compare/v1.0.133...v1.0.136#diff-e58acdc9a66e16df7f274469850f520c52ce04a516e49cecb5da85e55b53d708R16

@teor2345 @conradoplg @jvff @oxarbitrage @upbqdn wdyt?

[dconnolly]

'Requesting changes' to block for now

[teor2345]

This introduces a new Buf type that has an as_str method that uses unsafe:

This function is unsafe because it does not check that the bytes passed to it are valid UTF-8. If this constraint is violated, undefined behavior results, as the rest of Rust assumes that &strs are valid UTF-8.

https://doc.rust-lang.org/std/str/fn.from_utf8_unchecked.html#safety

This is safe because write_str makes sure that the whole string is always written to the buffer. Since the input strings are all valid UTF-8, and concatenated valid UTF-8 strings are also valid, the buffer is always valid UTF-8.

(A safety comment in serde would be nice though.)

[dconnolly]

This introduces a new Buf type that has an as_str method that uses unsafe:

This function is unsafe because it does not check that the bytes passed to it are valid UTF-8. If this constraint is violated, undefined behavior results, as the rest of Rust assumes that &strs are valid UTF-8.

https://doc.rust-lang.org/std/str/fn.from_utf8_unchecked.html#safety

This is safe because write_str makes sure that the whole string is always written to the buffer. Since the input strings are all valid UTF-8, and concatenated valid UTF-8 strings are also valid, the buffer is always valid UTF-8.

(A safety comment in serde would be nice though.)

Even though one can create an arbitrary slice of bytes with Buf::new()?

[teor2345]

Even though one can create an arbitrary slice of bytes with Buf::new()?

Nice catch!

Yes, this is a safety bug, because the initial bytes data doesn't have to be UTF-8.

Here's an example that I think shows the bug (0xfe is invalid in UTF-8 regardless of context):

let bytes = [0xfe];
let buf = Buf::new(&mut bytes);
let bad_str = buf.as_str();

Here is the reference I used for invalid UTF-8:
https://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt

But it's safe as currently used, because it is a private type that gets initialised with a zero-filled buffer:
serde-rs/serde@v1.0.133...v1.0.136#diff-3fe2f7b5e15c5b4043e41ea777df0b69f2e51947ce83a52fca7a0e53fce27699R1371-R1373

Did you want to open a bug against serde?

[conradoplg]

Should we make the bump since the bug can't be triggered externally, or block on this? @dconnolly would you like someone else to report the bug upstream?

[teor2345]

Should we make the bump since the bug can't be triggered externally, or block on this? @dconnolly would you like someone else to report the bug upstream?

I think it's fine to just merge this PR.

We're not reviewing standard library updates or transitive dependency updates in this level of detail, so I think we can be less detailed in our other dependency reviews.

[teor2345]

We'll need to mergifyio refresh this PR once the fix in PR #3705 merges.

[dconnolly]

mergifyio refresh

[dependabot]

Looks like serde is up-to-date now, so this is no longer needed.