-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SR-2804] URLProtocol.setProperty doesn't take URLRequest, instead requires NSMutableURLRequest #4324
Comments
Cloned out SR-2805 to fix the warning and possible mis-optimization. |
Comment by Mark Lilback (JIRA) I'm not sure if this should be filed as a separate bug, but if you circumvent the error as described above, then let value = URLProtocol.property(forKey: "example", in: request) will return nil instead of true |
Comment by aaron crespo (JIRA) Foundation documentation states: The Swift overlay to the Foundation framework provides the URLRequest structure, which bridges to the NSMutableURLRequest class and its immutable superclass, NSURLRequest. The URLRequest value type offers the same functionality as the NSMutableURLRequest reference type, and the two can be used interchangeably in Swift code that interacts with Objective-C APIs. This behavior is similar to how Swift bridges standard string, numeric, and collection types to their corresponding Foundation classes. Which is not the case with respect to NSURLConnectionDelegate and URLProtocol implementations. This issue is still present in Xcode 8.3 beta's most recent release. |
@phausler, who's responsible for URLRequest? That documentation seems a little fishy. |
(but I'm not completely sure) |
Foundation owns the bridge part for it. "which bridges to the NSMutableURLRequest class" the bridge is only to NSURLRequest |
now it so happens that we create a mutable version under the hood but there is no adopter of _ObjectiveCBridgeable for NSMutableURLRequest. The "safe" (but ugly) way to do this: var request = URLRequest(url: URL(string: "https://apple.com")!) |
Comment by aaron crespo (JIRA) One problem is that the cast to NSMutableRequest hides the mutation from the swift compiler so the following code produces a warning: typical URLProtocol implementation override func startLoading() {
var req = request
URLProtocol.setProperty(true, forKey: NetworkActivityKeys.HandledKey, in: (req as NSURLRequest).mutableCopy() as! NSMutableURLRequest)
//... Variable 'req' was never mutated; consider changing to 'let' constant
} Is the underlying NSMutableRequest created with |
Oh no that isn't doing what you were wanting, sorry. This is probably more along the lines of what you want:
|
we should refine the URLProtocol implementation, that is really disgusting. just a potential change (needs to go through API review etc) that would have to be made in the Foundation overlay
|
Comment by aaron crespo (JIRA) Yeah the ugliness was already tucked away hidden in a helper function but I figured I would make sure some ugly bridging behavior was pointed out somewhere. |
The problem with doing it as the work-around is that it is causing four copies where it should at most do one (and perhaps zero in the common case) |
Comment by aaron crespo (JIRA) is API review referring to "corelib" or "evolution"? |
It is in reference to the internal process for the Cocoa and software development teams maintaining the implementations backing that where we will follow a similar review process to swift-evolution. Primarily this is to get eyes on APIs for the stakeholders that will be responsible for supporting any API changes. |
Comment by aaron crespo (JIRA) Cool was curious if there was something someone outside could do to help this along. |
Comment by Andrea de Marco (JIRA) Hi, let request = URLRequest(url: URL(string: "https://apple.com")!)
URLProtocol.setProperty("bar", forKey: "foo", in: request as! NSMutableURLRequest)
let foo = URLProtocol.property(forKey: "foo", in: request)
//foo is nil Isn't it? |
That is the same failure, you cannot cast a structural type to a mutable type since they are not bridged. But you can cast a mutable to a structural type because the super-class is bridgeable. So your `as!` will halt. |
Comment by Andrea de Marco (JIRA) On Xcode 8.3 my snippet works, but with an unexpected result (nil). I think the problem is quite different, becoming URLRequest a structure the property inside the request is added to one of its copy. To be clear: until this fix, the URLProtocol property mechanins isn't usable. Right? |
It is usable but you would need to bridge back out to the mutable reference and back; via the snippet I posted earlier
|
I've run into this issue multiple times now. It would be great to see it fixed. The obvious solution here is to extend |
Environment
Xcode 8 / Swift 3 / macOS Sierra / iOS 10
Additional Detail from JIRA
md5: b6c1d5ebeca551022bac780f772ec9f8
cloned to:
Issue Description:
For example:
This throws an error:
Type casting circumvents the error:
But then a warning is thrown:
The text was updated successfully, but these errors were encountered: