Defensive access to MIMEType #15318
Defensive access to MIMEType #15318
Conversation
Prevent possible crashes
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a @try-@catch block to defensively access the MIMEType property of an NSURLResponse, preventing a potential crash. The change is straightforward and addresses the reported issue. I've added one suggestion to improve the implementation by logging the caught exception for better diagnostics, while also removing a redundant line of code.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| // Defensive access to MIMEType | ||
| NSString *mime = nil; | ||
| @try { | ||
| // Safely copy MIMEType to prevent use after free | ||
| mime = [response.MIMEType copy]; | ||
| } @catch (NSException *exception) { | ||
| FPRLogWarning(@"MIMETypeException", | ||
| @"Exception while accessing MIMEType for URL %@: %@. Trace will continue " | ||
| @"without MIMEType.", | ||
| self.URLRequest.URL, exception); | ||
| } |
There was a problem hiding this comment.
I don't believe a crash like this can be handled. Only NSExceptions can be handled and there isn't one being raised here. This crash is coming at a lower level.
There was a problem hiding this comment.
Would wrapping the 2 lines I already contributed in a if (response) for example would be a better solution? (of course keeping tabs if the crash ever shows up again)
There was a problem hiding this comment.
No, because in order for the getMIMEType method to appear in the stack trace, the response is non-nil. Because if the response was nil, the MIMEType call would be a no-op.
There was a problem hiding this comment.
Ok got it, I'm a bit out of ideas maybe I can do a bit more research and try a better solution for this
Thanks Nick
Prevent possible crashes adressing comments from #15317 and crash reported in #14734
Discussion
This addresses the comments for the proposed improvements not addressing the possible crash found when MIMEType is called by adding a little try catch and keeping the improvements made in the past pull request
@ncooke3 Hoping this is better!
Testing
API Changes
No API Changes