Fix readme to reflect the use of GitHub Actions #424
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Document the sound use of unsafe in HeaderValue::to_str().
The tests inlcude variation on the placement of unicode codepoints that encode to multi-byte UTF-8 and an instance of a [u8] with invalid UTF-8 that is still a valid HeaderValue.
The new code uses Iterator::try_fold() instead of external iteration to review the bytes in the HeaderValue and separate the printing of visible ascii bytes printing other bytes (and special casing b'"'). This new implementation may behave slightly different for a HeaderValue of length usize::MAX, but such a HeaderValue is likely to exhaust memory before the Debug impl comes into play (which also makes it hard to test this hypothetical edge case).
The comments describe the invariants that make the use of unsafe sound.
The simplification removes the need for one of the invariants that described how the use of unsafe is sound.
HeaderValue::from_maybe_shared_unchecked() is currently an unsafe function but it could be made a safe function without intoducing a soundness hole because it does not do anything directly that depends on `unsafe` and it isn't protecting any invariants of HeaderValue that are relied upon later when using `unsafe`. from_maybe_shared_unchecked() can accept bytes as a HeaderValue that are invalid bytes for what is described in RFC 7230 (section 3.2) but all such bytes would still be valid to convert directly to a str since they are all valid single-byte UTF-8.
HeaderValue::from_maybe_shared_unchecked had been marked unsafe but this was to indicate caller responsibility for api constraints and not caller responsibility for avoiding undefined behaviour. This change removes the use of unsafe for this purpose and instead uses a more explict doc comment to draw the caller's attention to the restrictions (as well as the continued use of the "_unchecked" suffix. This is not a breaking change (despite affecting the public API) because removing "unsafe" from a function still allows existing uses to compile. This use of "unsafe" was not protecting an invariant on the internal structre of HeaderValue that was relined on in unsafe blocks elsewhere in the module to ensure the absence of undefined behaviour. All (other) unsafe blocks in this module rely on local checking to ensure they avoid undefined behaviour.
Some of the comments describing the invariants necessary to ensure the sound use of "unsafe" refered to an older version of the reasoning that had used 2 invariants. This change makes them refer to "the invariant" instead of "invariant 1".
The "Build Status" markup badge had not been updated to refect the change of CI systems from TravisCI to GitHub Actions.
|
This was mistakenly submitted on top of #419. It was intended to be a stand-alone change against master so I will close this one and resubmit it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This updates the Build Status badge to reflect the move to GitHub Actions.