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 #77

Closed
castroteam opened this issue Sep 19, 2024 · 7 comments · Fixed by #78
Closed

Crash #77

castroteam opened this issue Sep 19, 2024 · 7 comments · Fixed by #78
Assignees
Labels
bug Something isn't working

Comments

@castroteam
Copy link

snippet.txt

The attached HTML snippet causes a crash

within MarkupNSAttributedStringVisitor#reduceBreaklineInResultNSAttributedString

NSMutableRLEArray replaceObjectsInRange:withObject:length:: Out of bounds

Probably the HTML is malformed, but ZMarkupParser should fail more gracefully.

Thanks!

@zhgchgli0718
Copy link
Member

will fix asap, thanks for your feedback

@zhgchgli0718 zhgchgli0718 self-assigned this Sep 19, 2024
@zhgchgli0718 zhgchgli0718 added the bug Something isn't working label Sep 19, 2024
@James-chok
Copy link

snippet.txt

The attached HTML snippet causes a crash

within MarkupNSAttributedStringVisitor#reduceBreaklineInResultNSAttributedString

NSMutableRLEArray replaceObjectsInRange:withObject:length:: Out of bounds

Probably the HTML is malformed, but ZMarkupParser should fail more gracefully.

Thanks!

Did it happen on iOS18?

@James-chok
Copy link

I found the parser works weirdly on iOS18. 🤔

@castroteam
Copy link
Author

Yes I think so.

@zhgchgli0718
Copy link
Member

Thank you all for your feedback and assistance.


private extension NSAttributedString.Key {
    static let breaklinePlaceholder: NSAttributedString.Key = .init("breaklinePlaceholder")
    struct BreaklinePlaceholder: OptionSet {
        let rawValue: Int

        static let tagBoundaryPrefix = BreaklinePlaceholder(rawValue: 1)
        static let tagBoundarySuffix = BreaklinePlaceholder(rawValue: 2)
        static let breaklineTag = BreaklinePlaceholder(rawValue: 3)
    }
}

The root cause is that the custom NSAttributedString.Key requires an extension from Hashable. Otherwise, on iOS 18 and above, it will throw the following error:

Obj-C `-hash` invoked on a Swift value of type `(extension in ZMarkupParser):__C.NSAttributedStringKey.(unknown context at $10380bcc0).BreaklinePlaceholder` that is Equatable but not Hashable; this can lead to severe performance problems.

This leads to a subsequent crash:

*** Terminating app due to uncaught exception 'NSRangeException', reason: 'NSMutableRLEArray replaceObjectsInRange:withObject:length:: Out of bounds'
*** First throw call stack:

However, the expected behavior is to record each breakline position and merge them according to the rules.

If we simply extend the struct with BreaklinePlaceholder: OptionSet, Hashable, the system will automatically merge repeated BreaklinePlaceholder, which is not what we want.

Therefore, I changed it to use a class declaration for the placeholder to maintain the original merging logic.

I also adjusted the crash point in mutableAttributedString.enumerateAttribute(.breaklinePlaceholder, in: NSMakeRange(0, totalLength)), implementing a safer approach by first collecting the ranges and then performing the deletions.

For more details, please refer to this PR. #78

@James-chok
Copy link

Thank you for fixing this crash!

@zhgchgli0718
Copy link
Member

the full story: https://en.zhgchg.li/posts/9e43897d99fc/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants