Bug 181390 - [Attachment Support] Expose file name and content type of WKAttachment
Summary: [Attachment Support] Expose file name and content type of WKAttachment
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks: 181420
  Show dependency tree / graph
 
Reported: 2018-01-08 09:14 PST by Wenson Hsieh
Modified: 2018-01-08 18:06 PST (History)
9 users (show)

See Also:

Attachments
Patch (41.78 KB, patch)
2018-01-08 09:47 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Fix non-macOS ports (47.04 KB, patch)
2018-01-08 10:18 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Remove AttachmentRequestOptions (39.38 KB, patch)
2018-01-08 12:07 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Add a URL message check in WebPageProxy (39.46 KB, patch)
2018-01-08 15:09 PST, Wenson Hsieh 		thorton: review+
Details | Formatted Diff | Diff
Patch for landing (39.41 KB, patch)
2018-01-08 17:33 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2018-01-08 09:14:18 PST 
<rdar://problem/36336837>
Comment 1 Wenson Hsieh 2018-01-08 09:47:39 PST 
Created attachment 330709 [details]
Patch
Comment 2 Wenson Hsieh 2018-01-08 10:18:31 PST 
Created attachment 330714 [details]
Fix non-macOS ports
Comment 3 Wenson Hsieh 2018-01-08 12:07:53 PST 
Created attachment 330719 [details]
Remove AttachmentRequestOptions
Comment 4 Tim Horton 2018-01-08 14:44:25 PST 
Comment on attachment 330719 [details]
Remove AttachmentRequestOptions

View in context: https://bugs.webkit.org/attachment.cgi?id=330719&action=review

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:2796
> +    if (!decoder.decode(info.filePath))
> +        return false;

Sending filesystem paths from a potentially-compromised Web Content process to the UI process scares me. Do we have to do this? How do we make it safe? Maybe look at MESSAGE_CHECK_URL for ideas? Keep a set of files that you sent or something?
Comment 5 Wenson Hsieh 2018-01-08 14:52:49 PST 
(In reply to Tim Horton from comment #4)
> Comment on attachment 330719 [details]
> Remove AttachmentRequestOptions
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=330719&action=review
> 
> > Source/WebKit/Shared/WebCoreArgumentCoders.cpp:2796
> > +    if (!decoder.decode(info.filePath))
> > +        return false;
> 
> Sending filesystem paths from a potentially-compromised Web Content process
> to the UI process scares me. Do we have to do this? How do we make it safe?
> Maybe look at MESSAGE_CHECK_URL for ideas? Keep a set of files that you sent
> or something?

Good point. Added a MESSAGE_CHECK_URL when receiving an AttachmentInfo.
Comment 6 Wenson Hsieh 2018-01-08 15:09:20 PST 
Created attachment 330744 [details]
Add a URL message check in WebPageProxy
Comment 7 Wenson Hsieh 2018-01-08 15:57:30 PST 
Comment on attachment 330744 [details]
Add a URL message check in WebPageProxy

View in context: https://bugs.webkit.org/attachment.cgi?id=330744&action=review

> Source/WebKit/UIProcess/WebPageProxy.cpp:5219
> +    if (!info.filePath.isEmpty())

I just realized that this check isn't necessary — I've removed it.
Comment 8 Wenson Hsieh 2018-01-08 17:33:01 PST 
Created attachment 330765 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2018-01-08 18:05:52 PST 
Comment on attachment 330765 [details]
Patch for landing

Clearing flags on attachment: 330765

Committed r226604: <https://trac.webkit.org/changeset/226604>