New issue
Advanced search Search tips

Issue 659649 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

Remove usage of UTF8String

Project Member Reported by jif@chromium.org, Oct 26 2016

Issue description

NSString's UTF8String can return null for some NSStrings, even if
|canBeConvertedToEncoding:NSUTF8StringEncoding| returns YES.
This has led to at least 2 crashes that were not encountered during
the development period.

Conclusion: We should stop using UTF8String.
 

Comment 1 by jif@chromium.org, Oct 26 2016

Note: olivierrobin is removing usage in spotlight.
Cc: justincohen@chromium.org marq@chromium.org rohitrao@chromium.org

Comment 3 by jif@chromium.org, Oct 26 2016

Note: Usage of method is getting banned using the presubmit scripts:
https://chromereviews.googleplex.com/531827014/
https://codereview.chromium.org/2450033004/

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 26 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/189ad1b2c0064e3ec3b29f4a96a374986bc2da48

commit 189ad1b2c0064e3ec3b29f4a96a374986bc2da48
Author: olivierrobin <olivierrobin@google.com>
Date: Wed Oct 26 15:13:19 2016

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6539870a0bbb33b28a1b56c85124874eb22c9e68

commit 6539870a0bbb33b28a1b56c85124874eb22c9e68
Author: jif <jif@chromium.org>
Date: Thu Oct 27 10:19:48 2016

Add UTF8String to the banned method list.

NSString's UTF8String can return null for some NSStrings, even if
|canBeConvertedToEncoding:NSUTF8StringEncoding| returns YES.
This has led to at least 2 crashes that were not encountered during
the development period.

BUG= 659649 , 656108, 653379

Review-Url: https://codereview.chromium.org/2450033004
Cr-Commit-Position: refs/heads/master@{#427991}

[modify] https://crrev.com/6539870a0bbb33b28a1b56c85124874eb22c9e68/PRESUBMIT.py

Comment 6 by vabr@chromium.org, Nov 3 2016

Labels: Postmortem-Followup
Marking as Postmortem-Followup because it tracks an action item to prevent repeating go/chromepostmortem284 in the future.
Components: UI>Internationalization
Project Member

Comment 9 by sheriffbot@chromium.org, Apr 11 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 10 by sczs@chromium.org, Apr 11 2018

Cc: -marq@chromium.org
Owner: marq@chromium.org
Status: Assigned (was: Untriaged)
marq@ do you think this is important to do now? 

Comment 11 by marq@chromium.org, Apr 18 2018

Status: Started (was: Assigned)
It looks like the number of usage sites is now tiny. I'll clean them up and then close this.
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/927fb69cef166919cc856a5ce1baeece3518cffd

commit 927fb69cef166919cc856a5ce1baeece3518cffd
Author: Mark Cogan <marq@google.com>
Date: Wed Apr 18 11:00:14 2018

[iOS] Remove remaining uses of UTF8String.

Bug:  659649 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I8813d293c71b1403f18368c60fa0a2435193cac4
Reviewed-on: https://chromium-review.googlesource.com/1016286
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551626}
[modify] https://crrev.com/927fb69cef166919cc856a5ce1baeece3518cffd/ios/net/nsurlrequest_util.mm
[modify] https://crrev.com/927fb69cef166919cc856a5ce1baeece3518cffd/ios/web/webui/crw_web_ui_page_builder.mm

Comment 13 by marq@chromium.org, Apr 18 2018

Status: Fixed (was: Started)

Sign in to add a comment