Bug 143599

Summary: [Content Filtering] Support modifying request URLs
Product: WebKit Reporter: Andy Estes <aestes>
Component: New BugsAssignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, ddkilzer, kling, mitz
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 128858    
Attachments:
Description Flags
Patch darin: review+

Description Andy Estes 2015-04-10 01:12:28 PDT
[Content Filtering] Support modifying request URLs
Comment 1 Andy Estes 2015-04-10 01:14:40 PDT
rdar://problem/20442560
Comment 2 Andy Estes 2015-04-10 01:30:44 PDT
Created attachment 250506 [details]
Patch
Comment 3 WebKit Commit Bot 2015-04-10 01:33:13 PDT
Attachment 250506 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/cocoa/NetworkExtensionContentFilter.mm:94:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Darin Adler 2015-04-10 09:20:48 PDT
Comment on attachment 250506 [details]
Patch

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

> Source/WebCore/platform/cocoa/NetworkExtensionContentFilter.mm:108
> +    NSURL *replacementNSURL = [NSURL URLWithString:replacementURLString.get()];

It’s possibly not OK to use URLWithString: instead of using one of the WebCoreNSURLExtras functions. Typically it would be URLWithUserTypedString although it doesn’t quite seem right in this case. URLWithString: doesn’t handle all the unusual characters correctly. But since we are really trying to make a URL, the best way is probably:

    URL replacementURL(URL(), replacementURLString.get());

I don’t think we need to explicitly convert to an NSURL on the way to creating a URL; the logic in WebCore::URL is likely what we want even for unusual characters.

It’s even possible we might want to use a base URL, but I suppose we only want to allow absolute URLs.

> Source/WebCore/testing/MockContentFilterSettings.h:29
> +#include "URL.h"

No reason to add this include.
Comment 5 Andy Estes 2015-04-10 12:01:37 PDT
Committed r182637: <http://trac.webkit.org/changeset/182637>