Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add ByteBuffer methods getUTF8ValidatedString and readUTF8ValidatedString #2973

Merged
merged 14 commits into from
Nov 21, 2024

Conversation

adam-fowler
Copy link
Contributor

Add methods to ByteBuffer to read validated UTF8 strings

Motivation:

The current readString and getString methods of ByteBuffer do not verify that the string being read is valid UTF8. The Swift 6 standard library comes with a new initialiser String(validating:as:). This PR adds alternative methods to ByteBuffer which uses this instead of String(decoding:as:).

Modifications:

Added ByteBuffer.getUTF8ValidatedString(at:length:)
Added ByteBuffer.readUTF8ValidatedString(length:)

Result:

You can read strings from a ByteBuffer and be certain they are valid UTF8

Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Thanks for this! I think the API design needs a bit of a tweak.

I also wonder if we can implement an older-Swift fallback using https://developer.apple.com/documentation/swift/string/init(validatingutf8:)-208fn.

@adam-fowler
Copy link
Contributor Author

Thanks for this! I think the API design needs a bit of a tweak.

I also wonder if we can implement an older-Swift fallback using https://developer.apple.com/documentation/swift/string/init(validatingutf8:)-208fn.

The problem with using the above function is it requires the string to be null terminated. I guess I could copy it out to a separate buffer and add the null termination.

@Lukasa
Copy link
Contributor

Lukasa commented Nov 14, 2024

@adam-fowler
Copy link
Contributor Author

Oh, we could do this one instead: https://developer.apple.com/documentation/swift/string/init(utf8string:)-8qmaq

Which also requires a null-terminated string

@adam-fowler adam-fowler requested a review from Lukasa November 20, 2024 13:51
@Lukasa Lukasa added the 🆕 semver/minor Adds new public API. label Nov 21, 2024
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Fantastic, I'm really happy with this. Thanks @adam-fowler!

@Lukasa
Copy link
Contributor

Lukasa commented Nov 21, 2024

Looks like there are a few compile errors in the unit tests on older Swift compilers.

@adam-fowler adam-fowler requested a review from Lukasa November 21, 2024 15:37
@Lukasa
Copy link
Contributor

Lukasa commented Nov 21, 2024

FWIW, I think it's usually better to put the compiler guards inside the test function, and throw XCTSkip otherwise to mark the test as explicitly skipped.

@adam-fowler
Copy link
Contributor Author

FWIW, I think it's usually better to put the compiler guards inside the test function, and throw XCTSkip otherwise to mark the test as explicitly skipped.

done

@Lukasa Lukasa enabled auto-merge (squash) November 21, 2024 17:28
@Lukasa Lukasa merged commit 74cf44e into apple:main Nov 21, 2024
42 of 43 checks passed
@adam-fowler adam-fowler deleted the read-validated-string branch November 21, 2024 17:47
Lukasa pushed a commit to Lukasa/swift-nio that referenced this pull request Nov 22, 2024
…ring (apple#2973)

Add methods to ByteBuffer to read validated UTF8 strings

### Motivation:

The current `readString` and `getString` methods of `ByteBuffer` do not
verify that the string being read is valid UTF8. The Swift 6 standard
library comes with a new initialiser `String(validating:as:)`. This PR
adds alternative methods to ByteBuffer which uses this instead of
`String(decoding:as:)`.

### Modifications:

Added `ByteBuffer.getUTF8ValidatedString(at:length:)`
Added `ByteBuffer.readUTF8ValidatedString(length:)`

### Result:

You can read strings from a ByteBuffer and be certain they are valid
UTF8

---------

Co-authored-by: Cory Benfield <lukasa@apple.com>
(cherry picked from commit 74cf44e)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants