Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

crash fix with iOS 7 stringByAddingPercentEncodingWithAllowedCharacters method #3028

Closed

Conversation

sftnhrd
Copy link
Contributor

@sftnhrd sftnhrd commented Oct 7, 2015

No description provided.

@kcharwood
Copy link
Contributor

Thanks @softenhard. This has been on my list to fix, but I hadn't yet thought about how to handle some of the double byte characters like emoji. I know relying on NSString length and enumeration can result in a split character. Any thoughts on that?

@sftnhrd
Copy link
Contributor Author

sftnhrd commented Oct 8, 2015

@kcharwood Oh yes, forgot about that while fast-fixing my project :)
So I've added a couple of lines to support multibyte characters.
Check it out:

static NSUInteger const batchSize = 50;

    NSInteger index = 0;
    NSMutableString *escaped = @"".mutableCopy;

    while (index < string.length) {
        NSUInteger length = MIN(string.length - index, batchSize);
        NSRange range = NSMakeRange(index, length);

        // To avoid breaking up character sequences such as 👴🏿
        range = [string rangeOfComposedCharacterSequencesForRange:range];

        NSString *substring = [string substringWithRange:range];
        NSString *encoded = [substring stringByAddingPercentEncodingWithAllowedCharacters:allowedCharacterSet];
        [escaped appendString:encoded];

        index += range.length;
    }

@kcharwood
Copy link
Contributor

What happens if the split is on the mutlibyte character? Is the character included in both results?

Could you add some unit tests demonstrating the fix? Thanks!

kcharwood added a commit that referenced this pull request Oct 13, 2015
- Added test demonstrating fix
@kcharwood kcharwood added this to the 2.6.1 milestone Oct 13, 2015
@kcharwood
Copy link
Contributor

Updated in 6b6d41d

@kcharwood
Copy link
Contributor

Merged with 6b6d41d

@kcharwood kcharwood closed this Oct 13, 2015
kcharwood added a commit that referenced this pull request Oct 13, 2015
- Added test demonstrating fix
@kcharwood
Copy link
Contributor

Merged in 3_0_0 with 039a317

kcharwood added a commit that referenced this pull request Oct 13, 2015
- Added test demonstrating fix
@sftnhrd sftnhrd deleted the fix-url-query-parameter-encoding branch October 14, 2015 06:35
sergiou87 pushed a commit to plexinc/AFNetworking that referenced this pull request Dec 12, 2015
- Added test demonstrating fix
@Coeur
Copy link
Contributor

Coeur commented Jun 1, 2017

As AFNetworking code is referencing this issue in https://github.com/AFNetworking/AFNetworking/blob/master/AFNetworking/AFURLRequestSerialization.m#L54, I'd like to quote in this pull request the details of this issue.

The reason and the workaround was discovered by @PrideChung and is a memory crash issue in stringByAddingPercentEncodingWithAllowedCharacters, reported in Alamofire/Alamofire#206. Scope of this issue was refined by @cnoon as only affecting iOS7 and iOS8, not affecting iOS9:

Batching is required for escaping due to an internal bug in iOS 8.1 and 8.2. Encoding more than a few hundred Chinese characters causes various malloc error crashes. To avoid this issue until iOS 8 is no longer supported, batching MUST be used for encoding. This introduces roughly a 20% overhead.

@CodeLife2012
Copy link

nice

@fighterLS
Copy link

fighterLS commented Feb 22, 2019

I get a crash in my application. like this
| Application Specific Information: *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[__NSCFString appendString:]: nil argument'
-- | --
0 | CoreFoundation | 0x0000000202a74ec4 ___exceptionPreprocess + 228
1 | libobjc.A.dylib | 0x0000000201c45a40 _objc_exception_throw + 56
2 | CoreFoundation | 0x000000020297b594 +[NSException raise:format:] + 116
3 | CoreFoundation | 0x00000002029ea7d8 _mutateError + 204
4 | homework | 0x0000000102ae95dc AFPercentEscapedStringFromString (AFURLRequestSerialization.m:73)
5 | homework | 0x0000000102ae9880 -[AFQueryStringPair URLEncodedStringValue] (AFURLRequestSerialization.m:108)
6 | homework | 0x0000000102ae9a8c AFQueryStringFromParameters (AFURLRequestSerialization.m:0)
7 | homework | 0x0000000102aec560 -[AFHTTPRequestSerializer requestBySerializingRequest:withParameters:error:] (AFURLRequestSerialization.m:0)
8 | homework | 0x0000000102aeb878 -[AFHTTPRequestSerializer requestWithMethod:URLString:parameters:error:] (AFURLRequestSerialization.m:376)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants