Skip to content

Conversation

@memoto
Copy link
Contributor

@memoto memoto commented Oct 22, 2024

Swift 6 String's init?(data: Data, encoding: String.Encoding) fails validation during encoding utf8 data to ascii and returns nil.

As proposed solution we inspect memory buffer bound to UInt8/CChar bytes, assuming they are null-terminted which allows us to produce a new string by simply copying them and interpreting as ascii.

Copy link
Contributor

@mollyIV mollyIV 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 creating this pull request <3

I've been testing the changes on one of personal projects and can confirm it's working as expected - I can get the activity log parsed correctly.

Prior to these changes, I've been getting an error complaining about incorrect activity log file. See #219 for more details.

Can we please make the CI checks green? 🙏

Other than that, let's ship it :shipit:

aleksandergrzyb
aleksandergrzyb previously approved these changes Dec 9, 2024
@frugoman
Copy link

Any reason why this one's not getting attention? does seem like the fix needed.

@memoto
Copy link
Contributor Author

memoto commented Dec 16, 2024

@polac24 @ecamacho @CognitiveDisson Waiting for your approve 🙏

@aleksandergrzyb
Copy link
Collaborator

There are some linting issues 👀

Co-authored-by: Daeho Ro <[email protected]>
Signed-off-by: memoto <[email protected]>
@memoto
Copy link
Contributor Author

memoto commented Dec 19, 2024

@daeho-ro Thanks for suggestion that has fixed the linting!

@memoto memoto requested a review from daeho-ro December 19, 2024 10:54
@memoto
Copy link
Contributor Author

memoto commented Dec 20, 2024

@aleksandergrzyb Your stale review were dismissed due to lint fix, could you please approve again?

@aleksandergrzyb aleksandergrzyb merged commit 7cf8c50 into MobileNativeFoundation:master Dec 23, 2024
4 checks passed
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.

5 participants