[SR-2804] URLProtocol.setProperty doesn't take URLRequest, instead requires NSMutableURLRequest

[swift-ci]

Previous ID	SR-2804
Radar	rdar://problem/31244651
Original Reporter	andrewtheis (JIRA User)
Type	Bug

Environment

Xcode 8 / Swift 3 / macOS Sierra / iOS 10

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 4 | |Component/s | Foundation | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: b6c1d5ebeca551022bac780f772ec9f8

cloned to:

• SR-2805 Cast from value type to reference type subclass reported as "always fails"

Issue Description:

For example:

import Foundation

var request = URLRequest(url: URL(string: "https://apple.com")!)
URLProtocol.setProperty(true, forKey: "example", in: request)

This throws an error:

Cannot convert value of type 'URLRequest' to expected argument type 'NSMutableURLRequest'

Type casting circumvents the error:

import Foundation

let request = URLRequest(url: URL(string: "https://apple.com")!)

guard let mutableRequest = request as? NSMutableURLRequest else {
    exit(0)
}

URLProtocol.setProperty(true, forKey: "example", in: mutableRequest)

But then a warning is thrown:

Cast from 'URLRequest' to unrelated type 'NSMutableURLRequest' always fails

[belkadan]

Cloned out SR-2805 to fix the warning and possible mis-optimization.

[swift-ci]

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

[swift-ci]

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.

[belkadan]

@phausler, who's responsible for URLRequest? That documentation seems a little fishy.

[belkadan]

(but I'm not completely sure)

[phausler]

Foundation owns the bridge part for it. "which bridges to the NSMutableURLRequest class" the bridge is only to NSURLRequest

[phausler]

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")!)
URLProtocol.setProperty(true, forKey: "example", in: (request as.mutableCopy() as) NSMutableURLRequest)

[swift-ci]

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 let req = request? does it then become unsafe (IUO on cast failure)? The documentation says these may be used interchangeably

[phausler]

Oh no that isn't doing what you were wanting, sorry.

This is probably more along the lines of what you want:

        var request = URLRequest(url: URL(string: "https://apple.com")!)
        let mutableRequest = (request as NSURLRequest).mutableCopy() as! NSMutableURLRequest
        URLProtocol.setProperty(true, forKey: "example", in: mutableRequest)
        request = mutableRequest as URLRequest

[phausler]

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

extension URLProtocol {
    public class func setProperty(_ value: Any, forKey key: String, in request: inout URLRequest) {
        request._applyMutation {
            URLProtocol.setProperty(value, forKey: key, in: $0)
        }
    }

    public class func removeProperty(forKey key: String, in request: inout URLRequest) {
        request._applyMutation {
            URLProtocol.removeProperty(value, forKey: key, in: $0)
        }
    }
}

[swift-ci]

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.

[phausler]

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)

[swift-ci]

Comment by aaron crespo (JIRA)

is API review referring to "corelib" or "evolution"?

[phausler]

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.

[swift-ci]

Comment by aaron crespo (JIRA)

Cool was curious if there was something someone outside could do to help this along.

[swift-ci]

Comment by Andrea de Marco (JIRA)

Hi,
I guess the issue I found is related to this one:

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?

[phausler]

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.

[swift-ci]

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?

[phausler]

It is usable but you would need to bridge back out to the mutable reference and back;

via the snippet I posted earlier

        var request = URLRequest(url: URL(string: "https://apple.com")!)
        let mutableRequest = (request as NSURLRequest).mutableCopy() as! NSMutableURLRequest
        URLProtocol.setProperty(true, forKey: "example", in: mutableRequest)
        request = mutableRequest as URLRequest

[05262b81-54a9-4fe1-bf6a-96f8042de10e]

I've run into this issue multiple times now. It would be great to see it fixed. The obvious solution here is to extend URLProtocol to have overloads for the setProperty/removeProperty methods that work on inout URLRequest.