Skip to content
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.

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

Reject trailing bytes #198

Merged
merged 3 commits into from May 26, 2020
Merged

Reject trailing bytes #198

merged 3 commits into from May 26, 2020

Conversation

@ZoeyR
Copy link
Collaborator

ZoeyR commented Jul 24, 2017

Closes #193

There were two design possibilities I considered for tackling this problem. The current one changes the impls on SliceReader to be impls for &mut SliceReader. This allows me to access it after passing it to the deserializer. The other option would be to allow pub(restricted) access to the reader field of the deserializer and access it from there.

@ZoeyR ZoeyR force-pushed the ZoeyR:reject-trailing branch from 18b29b3 to ca80f4a Mar 14, 2020
@ZoeyR
Copy link
Collaborator Author

ZoeyR commented Mar 14, 2020

Finally got around to updating this to current bincode after 2.5 years 😅

src/de/read.rs Outdated Show resolved Hide resolved
@ZoeyR
Copy link
Collaborator Author

ZoeyR commented Mar 18, 2020

I've realized that this might be considered a breaking change.

@strega-nil
Copy link
Contributor

strega-nil commented Mar 18, 2020

I was the person who was saying that this should be considered a breaking change, fwiw.

ZoeyR added 2 commits Jul 23, 2017
@ZoeyR ZoeyR force-pushed the ZoeyR:reject-trailing branch from ca80f4a to 67e10ac May 24, 2020
@codecov-commenter
Copy link

codecov-commenter commented May 24, 2020

Codecov Report

Merging #198 into master will increase coverage by 0.08%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #198      +/-   ##
==========================================
+ Coverage   61.33%   61.42%   +0.08%     
==========================================
  Files          10       11       +1     
  Lines         807      827      +20     
==========================================
+ Hits          495      508      +13     
- Misses        312      319       +7     
Impacted Files Coverage Δ
src/config/legacy.rs 0.00% <ø> (ø)
src/de/mod.rs 77.77% <ø> (ø)
src/lib.rs 75.00% <ø> (ø)
src/config/mod.rs 40.35% <28.57%> (+0.35%) ⬆️
src/config/trailing.rs 57.14% <57.14%> (ø)
src/de/read.rs 54.54% <100.00%> (+1.71%) ⬆️
src/internal.rs 97.22% <100.00%> (+0.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 131b2b2...7188d6b. Read the comment docs.

@ZoeyR
Copy link
Collaborator Author

ZoeyR commented May 24, 2020

@ZoeyR ZoeyR merged commit a969be4 into servo:master May 26, 2020
9 checks passed
9 checks passed
Check (stable)
Details
Check (beta)
Details
Check (nightly)
Details
Check (1.18.0)
Details
Test (stable)
Details
Test (beta)
Details
Test (nightly)
Details
Lints
Details
Code Coverage
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.