| Summary: | [iOS] Attempting to open a Keynote document to iCloud.com from iCloud Files causes crash | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||||
| Component: | WebKit Misc. | Assignee: | Joseph Pecoraro <joepeck> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | ap, bfulgham, commit-queue, joepeck, pvollan, webkit-bug-importer, wenson_hsieh | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Joseph Pecoraro
2019-01-15 12:13:49 PST
Created attachment 359190 [details]
[PATCH] Proposed Fix
Comment on attachment 359190 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=359190&action=review > Source/WebKit/Shared/ios/WebIconUtilities.mm:101 > + if (!interactionController.icons.count) This check seems a bit strange. According to https://developer.apple.com/documentation/uikit/uidocumentinteractioncontroller/1616801-icons?language=objc, the resulting icons should be non-empty. I wonder if <rdar://problem/37157837> has resurfaced... > I wonder if <rdar://problem/37157837> has resurfaced... Likely. I filed <rdar://problem/47292126> which seems related. Should we bother trying to protect against this or rely on other components to make a fix? (In reply to Joseph Pecoraro from comment #4) > > I wonder if <rdar://problem/37157837> has resurfaced... > > Likely. I filed <rdar://problem/47292126> which seems related. > > Should we bother trying to protect against this or rely on other components > to make a fix? Hm...IMO, we should keep this in the backburner, and wait to see if the relevant folks are willing to pull <rdar://problem/47292126> into the relevant release. If that bug is punted, then it's probably for the better to land this as a mitigation (preferably with a comment citing that the check shouldn't be needed once <rdar://problem/47292126> is addressed). Created attachment 360210 [details]
[PATCH] Proposed Fix
Comment on attachment 360210 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=360210&action=review > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:173 > (allow mach-lookup > (xpc-service-name "com.apple.lsdiconservice") ;; Remove this line after <rdar://problem/47151295> is fixed. > (xpc-service-name "com.apple.iconservice")) I'm not sure if we should get rid of these at this point, but I'm leaving them to be safe. Comment on attachment 360210 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=360210&action=review > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:175 > + (global-name "com.apple.iconservices")) I think the prior line might be a typo: "com.apple.iconservice" -> "com.apple.iconservice". Can you try just adding the 's' to icon service and see if it fixes it? (In reply to Brent Fulgham from comment #8) > Comment on attachment 360210 [details] > I think the prior line might be a typo: "com.apple.iconservice" -> > "com.apple.iconservice". See! I did it again! "com.apple.iconservice" -> "com.apple.iconservices". (In reply to Brent Fulgham from comment #8) > Comment on attachment 360210 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=360210&action=review > > > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:175 > > + (global-name "com.apple.iconservices")) > > I think the prior line might be a typo: "com.apple.iconservice" -> > "com.apple.iconservice[s]". This does not solve the issue. Note that even though there is: (xpc-service-name "com.apple.lsdiconservice") There is also (further down): (global-name "com.apple.lsdiconservice") --- Would you rather I move the addition: (allow mach-lookup (global-name "com.apple.iconservices")) Down into the section at the bottom which has "with report": (allow mach-lookup (with report) ...) (In reply to Joseph Pecoraro from comment #10) > (In reply to Brent Fulgham from comment #8) > > Comment on attachment 360210 [details] > > [PATCH] Proposed Fix > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=360210&action=review > > > > > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:175 > > > + (global-name "com.apple.iconservices")) > > > > I think the prior line might be a typo: "com.apple.iconservice" -> > > "com.apple.iconservice[s]". > > This does not solve the issue. Okay. Thank you for checking. Comment on attachment 360210 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=360210&action=review >>>> Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:175 >>>> + (global-name "com.apple.iconservices")) >>> >>> I think the prior line might be a typo: "com.apple.iconservice" -> "com.apple.iconservice". >>> >>> Can you try just adding the 's' to icon service and see if it fixes it? >> >> This does not solve the issue. >> >> Note that even though there is: >> >> (xpc-service-name "com.apple.lsdiconservice") >> >> There is also (further down): >> >> (global-name "com.apple.lsdiconservice") >> >> --- >> >> Would you rather I move the addition: >> >> (allow mach-lookup >> (global-name "com.apple.iconservices")) >> >> Down into the section at the bottom which has "with report": >> >> (allow mach-lookup (with report) >> ...) > > Okay. Thank you for checking. Can you please change the incorrect string: (xpc-service-name "com.apple.iconservice") To the correct service name: (xpc-service-name "com.apple.iconservices") ... and then add your new rule for global-name? (allow mach-lookup (global-name "com.apple.iconservices")) (In reply to Joseph Pecoraro from comment #10) > Would you rather I move the addition down into the section at the bottom which has "with report": No, please leave it separate. The 'with-report' section is for something Per Arne and I are working on. Thanks! Created attachment 360376 [details]
[PATCH] For Landing
Comment on attachment 360376 [details] [PATCH] For Landing Clearing flags on attachment: 360376 Committed r240606: <https://trac.webkit.org/changeset/240606> |