Skip to content

Conversation

@TheBlueMatt
Copy link
Collaborator

I missed that this wasn't fully resolved in #919 (comment)

let error_details = match String::from(contents) {
// Check that the string is all-ASCII with no control characters before returning
// it.
Ok(s) if s.as_bytes().all(|c| c <= 0x7e && c >= 0x20) => s,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use is_ascii and !is_ascii_control on u8 data before converting.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah! I spent an embarrassing amount of time the other day looking for that, but I guess mostly annoyed that that isn't exposed on str and only u8.

@TheBlueMatt TheBlueMatt force-pushed the 2021-05-no-control-chars branch from 81ff7c8 to 1c4c910 Compare May 13, 2021 19:42
@TheBlueMatt TheBlueMatt force-pushed the 2021-05-no-control-chars branch from 1c4c910 to f0084e1 Compare May 13, 2021 21:58
@codecov
Copy link

codecov bot commented May 13, 2021

Codecov Report

Merging #921 (f0084e1) into main (4f880c6) will decrease coverage by 0.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #921      +/-   ##
==========================================
- Coverage   90.48%   90.46%   -0.02%     
==========================================
  Files          59       59              
  Lines       29788    29789       +1     
==========================================
- Hits        26953    26950       -3     
- Misses       2835     2839       +4     
Impacted Files Coverage Δ
lightning-block-sync/src/http.rs 95.12% <66.66%> (+0.01%) ⬆️
lightning/src/ln/functional_tests.rs 96.82% <0.00%> (-0.07%) ⬇️

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 4f880c6...f0084e1. Read the comment docs.

@TheBlueMatt TheBlueMatt merged commit 0b1f0a7 into lightningdevkit:main May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants