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

SR-3052. Incorrect string result for String methods with non latin symbols #706

Merged
merged 3 commits into from
Nov 3, 2016

Conversation

naithar
Copy link
Contributor

@naithar naithar commented Nov 2, 2016

Fixes Issue: https://bugs.swift.org/browse/SR-3052
Added tests that cover this issue.
Implemented possible fix by providing correct UTF8-length for _CFStringCreateByAddingPercentEncodingWithAllowedCharacters and _CFStringCreateByRemovingPercentEncoding

Also switched to CFStringGetLengthUTF8 method for string length where UTF-8 encoding is used.

@parkera
Copy link
Contributor

parkera commented Nov 2, 2016

Can we fix this issue without introducing a new length function for CFString? There is no "CFStringGetLengthUTF8" function on Darwin and I don't think we should introduce one. CFString has the CFStringGetMaximumSizeForEncoding function for dealing with differently-sized buffers when the string is encoded into something besides its native representation.

@naithar
Copy link
Contributor Author

naithar commented Nov 2, 2016

I'll look into this more. Thanks for the tip on CFStringGetMaximumSizeForEncoding.

@naithar
Copy link
Contributor Author

naithar commented Nov 2, 2016

@parkera updated fix. Tests is passing.

@parkera
Copy link
Contributor

parkera commented Nov 2, 2016

Thanks!

@parkera
Copy link
Contributor

parkera commented Nov 2, 2016

@swift-ci please test

@parkera
Copy link
Contributor

parkera commented Nov 3, 2016

Ok, don't know why the tests are stuck. Trying again.

@parkera
Copy link
Contributor

parkera commented Nov 3, 2016

@swift-ci please test

@naithar
Copy link
Contributor Author

naithar commented Nov 3, 2016

Looks like everything is ok now

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.

2 participants