Summary: | Delegate some read-only DOM properties to FrameLoaderClient and ChromeClient | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robert Hogan <robert> | ||||||
Component: | New Bugs | Assignee: | Robert Hogan <robert> | ||||||
Status: | RESOLVED WONTFIX | ||||||||
Severity: | Normal | CC: | abarth, ap, buildbot, cmarcelo, darin, dglazkov, eric, gustavo.noronha, gustavo, mikeperry-webkit, morrita, robert, webkit-ews, webkit.review.bot, xan.lopez | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | OS X 10.5 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 41801, 56678 | ||||||||
Attachments: |
|
Description
Robert Hogan
2011-03-13 15:23:58 PDT
Created attachment 85625 [details]
Patch
Attachment 85625 [details] did not build on qt: Build output: http://queues.webkit.org/results/8176088 Attachment 85625 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8181018 Attachment 85625 [details] did not build on win: Build output: http://queues.webkit.org/results/8176093 Attachment 85625 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8176104 Attachment 85625 [details] did not build on gtk: Build output: http://queues.webkit.org/results/8176106 Attachment 85625 [details] did not build on mac: Build output: http://queues.webkit.org/results/8175109 Comment on attachment 85625 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=85625&action=review > Source/WebCore/loader/FrameLoaderClient.h:235 > + virtual String referrer(const String& referrer) {return referrer;}; > + virtual String windowName(const String& name) {return name;}; > + virtual unsigned historyLength(unsigned length) {return length;}; No need for ; after these lines. Created attachment 85705 [details]
Patch
> Created an attachment (id=85705) [details]
> Patch
Just for curious, is it possible to have "drop in" support for fingerprint prevention?
It would be great if we can share the mechanism between ports.
(In reply to comment #10) > > Created an attachment (id=85705) [details] [details] > > Patch > > Just for curious, is it possible to have "drop in" support for fingerprint prevention? > It would be great if we can share the mechanism between ports. Hi Morita, If there are items for which the 'right thing to do' can be agreed on then there's no reason why WebKit couldn't implement them. I'm trying to build up a picture of what those might be at: https://trac.webkit.org/wiki/Fingerprinting Comment on attachment 85705 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=85705&action=review Drive-by comment: > Source/WebCore/loader/FrameLoader.cpp:3365 > String FrameLoader::referrer() const > { > - return m_documentLoader ? m_documentLoader->request().httpReferrer() : ""; > + return m_documentLoader ? m_client->referrer(m_documentLoader->request().httpReferrer()) : ""; > +} I'm pretty sure that this will only change the referrer as displayed to javascript, not what is sent over the network. Is that intentional? I feel like if we're going to hide referrers, we should hide it in both HTTP and JS, not just one or the other. (In reply to comment #12) > (From update of attachment 85705 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=85705&action=review > > Drive-by comment: > > > Source/WebCore/loader/FrameLoader.cpp:3365 > > String FrameLoader::referrer() const > > { > > - return m_documentLoader ? m_documentLoader->request().httpReferrer() : ""; > > + return m_documentLoader ? m_client->referrer(m_documentLoader->request().httpReferrer()) : ""; > > +} > > I'm pretty sure that this will only change the referrer as displayed to javascript, not what is sent over the network. That's correct, yes. > > Is that intentional? I feel like if we're going to hide referrers, we should hide it in both HTTP and JS, not just one or the other. It's intentional insofar as there is already a mechanism, in some ports at least, for the client to manipulate HTTP headers. In Qt, this is QNetworkAccessManager::createRequest() - which admittedly sits outside WebKit in Qt's case. The other factor is that there may be a better 'generic' rule for making the referrer header more privacy-sensitive in all cases. This is tracked at https://bugs.webkit.org/show_bug.cgi?id=51638 It would probably be quite confusing that this client call only affects DOM APIs. Comment on attachment 85705 [details]
Patch
I really don't like this design.
I think we should have a new AntiFingerprinting class or something that we can optionally pass these values to and have it reply with lies.
I don't want to plumb all of these up through the client in every different callsite. Instead, it would be better to plumb through this lier class, which can plumb through the client on ports which care. Then all the anti-fingerprinting logic can be implemented inside WebCore too.
What I dislike so much about this design is that previously all these simple data values are self-contained inside WebCore. Now they're needlessly piping up through WebKit, which is bad. Only in the anti-fingerprinting case do we need to ask WebKIt. And even then we only need to ask it if we've turne don the anti-fingerprinting stuff, and then the anti-fingerprinting code (which should be inside WebCore) can do the lying. |