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

Crash chuzzling an NSBigMutableString #3

Closed
tgaul opened this issue Aug 8, 2014 · 3 comments
Closed

Crash chuzzling an NSBigMutableString #3

tgaul opened this issue Aug 8, 2014 · 3 comments

Comments

@tgaul
Copy link

tgaul commented Aug 8, 2014

We encountered a crash in ChuzzleKit (as used by PromiseKit) when URL encoding a string that came out of a text field. The issue here is that for performance reasons, UIKit hands out a pointer to the internal mutable string representation (which can be an NSBigMutableString for long strings) and we were handing that to PromiseKit.

To fix this in our app, we made an immutable copy of the string we get from the text field and use that to call PromiseKit.

However, it would be best if PromiseKit didn't cause a crash if someone does this.

The 'crash' is the following exception at line 16 of Chuzzle.m (the mutable string copy line [(id)self setString:s];):

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', 
reason: 'Attempt to mutate immutable object with replaceCharactersInRange:withString:'

You'd think an NSBigMutableString would be mutable based on its name, but apparently not in all cases (at least you can't always call setString: on it without this happening).

@mxcl
Copy link
Owner

mxcl commented Aug 8, 2014

Hmm. I guess we could call mutableCopy in the mutable path. Hah.

Otherwise we can remove the mutable paths. They aren't important IMO. They were added in a pull request.

I've tested this kind of thing with mutableArray, and IIRC the objects respond to the mutable selectors, but they crash if you call them, so it's a strange area of objc.

Since we are categories we can't have a category for NSString and NSMutableString, or at least, in my testing, this did not work, hence the strange checks for mutability in the methods we have.

I'm leaning towards removing the mutability parts, I think it is not possible to properly support them.

@mxcl
Copy link
Owner

mxcl commented Aug 8, 2014

Also, you didn't want the textField’s string to be chuzzled anyway! I'm not sure if we should consider this ChuzzleKit’s fault or UIKit’s though.

Still I feel sticking with immutability is what I intended for this library, and what people expect.

@tgaul
Copy link
Author

tgaul commented Aug 9, 2014

Yeah, I hadn't realized it returned a pointer to it's representation string, so I fixed that in our code by making an immutable copy. It's something I'll be sure to remember in the future when using UITextView.

I think ideally chuzzle would avoid mutating objects in place, as that would seem to be safest.

I'm personally not sure about how it does whitespace trimming either. What's happening in my case is the string that's a URL parameter was a user-entered value in a text field. It turns out that I do my own whitespaces trimming, but who's to say that some app doesn't want to include spaces or newlines or tabs at the beginning/end of the string it's passing as a URL parameter?

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

No branches or pull requests

2 participants