Skip to content

JSONDecoder.readString(): throw on invalid UTF-8 #4675

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

Merged
merged 2 commits into from
Mar 27, 2023

Conversation

ruihe774
Copy link
Contributor

@ruihe774 ruihe774 commented Dec 7, 2022

String(decoding:as:), which previously used by
JSONDecoder.readString() to make string from JSON bytes, would silently repair invalid UTF-8 sequence and not throw a error.
This commit uses String(bytes:encoding:) instead,
which will fail on invalid UTF-8 seqence.
This matches the behavior of Darwin Foundation.

(Some examples of invalid UTF-8: http://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt)

String(decoding:as:), which previously used by
JSONDecoder.readString() to make string from JSON bytes,
would silently repair invalid UTF-8 sequence and not
throw a error.
This commit uses String(bytes:encoding:) instead,
which will fail on invalid UTF-8 seqence.
This matches the behavior of Darwin Foundation.

(Some examples of invalid UTF-8: http://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt)
@parkera parkera requested a review from kperryua December 7, 2022 14:34
@MaxDesiatov
Copy link
Contributor

@swift-ci test

@MaxDesiatov
Copy link
Contributor

@swift-ci test Windows

@MaxDesiatov
Copy link
Contributor

@swift-ci test

@ruihe774
Copy link
Contributor Author

ruihe774 commented Dec 15, 2022 via email

Copy link
Contributor

@kperryua kperryua left a comment

Choose a reason for hiding this comment

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

Changes look fine.

Something seems wrong infrastructurally in the CI:

    dyld: Library not loaded: @rpath/lib_SwiftSyntaxMacros.dylib
      Referenced from: /Users/ec2-user/jenkins/workspace/swift-corelibs-foundation-PR-macOS/branch-main/build/Ninja-ReleaseAssert/toolchain-macosx-x86_64/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swiftc
      Reason: image not found

@kperryua
Copy link
Contributor

@shahmishal Can you help out with the build failiure?

@MaxDesiatov

This comment was marked as duplicate.

4 similar comments
@MaxDesiatov

This comment was marked as duplicate.

@MaxDesiatov

This comment was marked as duplicate.

@shahmishal
Copy link
Member

@swift-ci test macOS

@MaxDesiatov
Copy link
Contributor

@swift-ci test macOS

@kperryua
Copy link
Contributor

Thank you! Sorry for the long delay while we figured out the CI issues.

@kperryua kperryua merged commit bafd3d0 into swiftlang:main Mar 27, 2023
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.

4 participants