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

Enforce escape functions to operate over UTF-8 #423

Merged
merged 4 commits into from
Jul 17, 2022
Merged

Conversation

Mingun
Copy link
Collaborator

@Mingun Mingun commented Jul 17, 2022

They are always operate over UTF-8 input, but that was not reflected in types.

This set of changes is more isolated that one in #422, and it also does not enforce owning of data in some cases where that is not required.

@Mingun Mingun added the encoding Issues related to support of various encodings of the XML documents label Jul 17, 2022
@Mingun Mingun requested a review from dralley July 17, 2022 11:35
@@ -901,11 +897,13 @@ impl<'a> BytesCData<'a> {
/// | `&` | `&amp;`
/// | `'` | `&apos;`
/// | `"` | `&quot;`
pub fn escape(self) -> BytesText<'a> {
BytesText::from_escaped(match escape(&self.content) {
pub fn escape(self, decoder: Decoder) -> Result<BytesText<'a>> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because BytesText::escape and BytesText::partial_escape introduced only in new 0.24 release, the changes in their signatures are not reflected in the changelog. Moreover, the next PRs will change that signature back

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

@codecov-commenter
Copy link

Codecov Report

Merging #423 (1a1f4bf) into master (d34b779) will decrease coverage by 0.03%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master     #423      +/-   ##
==========================================
- Coverage   49.53%   49.50%   -0.04%     
==========================================
  Files          22       22              
  Lines       13856    13861       +5     
==========================================
- Hits         6864     6862       -2     
- Misses       6992     6999       +7     
Flag Coverage Δ
unittests 49.50% <80.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
benches/microbenches.rs 0.00% <0.00%> (ø)
src/events/mod.rs 68.20% <33.33%> (-0.47%) ⬇️
src/de/mod.rs 77.86% <100.00%> (-0.46%) ⬇️
src/escapei.rs 12.15% <100.00%> (+0.21%) ⬆️
src/events/attributes.rs 94.10% <100.00%> (+<0.01%) ⬆️
src/reader.rs 60.13% <100.00%> (ø)
src/se/mod.rs 93.81% <100.00%> (ø)
src/writer.rs 49.28% <100.00%> (ø)

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 d34b779...1a1f4bf. Read the comment docs.

@dralley dralley merged commit 068b36e into tafia:master Jul 17, 2022
@Mingun Mingun deleted the escape branch July 17, 2022 14:38
@adrian17
Copy link

FYI, pretty sure that 1a1f4bf doesn't work, as equality comparison on Cow ignores the variant. You probably need to use matches instead?

@Mingun
Copy link
Collaborator Author

Mingun commented Jan 28, 2023

Yes, you're right. As far as I remember, there are a few more places in tests where we rely on such behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
encoding Issues related to support of various encodings of the XML documents
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants