Skip to content

[swift-corelibs-foundation]-- Fixed JSON Deserialisation of string values with spaces at the start String #263

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 1 commit into from
Apr 28, 2016

Conversation

salmanjamil
Copy link

currently for strings that had spaces at the start, the spaces were ignored during deserialisation process. e.g "{"title" : " hello world!!" }" would deserialise into ["title" : "hello world!!"] instead of ["title" : " hello world!"] skipping the space at the start of hello world. This was due to func consumeStructure(ascii: UInt8, input: Index) throws -> Index? consuming white spaces at the start and end of every structure. The fix is just to consume the starting whitespace sequence in case of QuotationMark Structure.

@argon
Copy link
Contributor

argon commented Feb 20, 2016

I think it would be preferable to treat this as a special case within parseString rather than complicate the consumeStructure function

func parseString(input: Index) throws -> (String, Index)? {
    guard let beginIndex = try consumeWhitespace(input).flatMap(consumeASCII(Structure.QuotationMark)) else {
        return nil
    }

@parkera parkera merged commit e96c418 into swiftlang:master Apr 28, 2016
@parkera
Copy link
Contributor

parkera commented Apr 28, 2016

Let's get the bug fix in now and fix up the style next.

@argon
Copy link
Contributor

argon commented Apr 28, 2016

Great stuff. I'll get a pull request with the change opened shortly.

@argon
Copy link
Contributor

argon commented Apr 28, 2016

☝️ 👉 #341

atrick pushed a commit to atrick/swift-corelibs-foundation that referenced this pull request Jan 12, 2021
[build-script] Add sanitizer flags
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