Alamofire: Space in URL causes crash

[tosbaha]

I am testing master branch with Swift 1.2 on Xcode 6.4. When Url has space inside it crashes. Example usage

let url = "http://www.example.com/abc/def.asp?k=abc def"

self.mngr.request(.GET, url)

Crashing function:

// MARK: - Convenience
func URLRequest(method: Method, URLString: URLStringConvertible, headers: [String: String]? = nil) -> NSMutableURLRequest {
    let mutableURLRequest = NSMutableURLRequest(URL: NSURL(string: URLString.URLString)!) //crash!
    mutableURLRequest.HTTPMethod = method.rawValue 

    if let headers = headers {
        for (headerField, headerValue) in headers {
            mutableURLRequest.setValue(headerValue, forHTTPHeaderField: headerField)
        }
    }
    return mutableURLRequest
}

If I use parameters and use it like below

let url = "http://www.example.com/abc/def.asp"
let parameters = ["k":"abc def"]
self.mngr.request(.GET, url,parameters:parameters)

It doesn't crash.

[kylef]

I think this crash comes from NSURL(string: URLString.URLString)! since you're passing in an invalid URL (URLs may not contain spaces).

URLs should be urlencoded and thus any spaces should be replaced with %20. The second example you have shown is properly encoding and escaping characters and thus doesn't crash.

[tosbaha]

Alamofire properly encodes if it is given as parameters. However it is not always possible to use parameters for every request. I believe that Alamofire should URL encode give string. Since Alamofire wants String as input and it converts to URL thanks to URLStringConvertible. If Alamofire required URL as input, then it was the responsibility of developer to give proper URL. Instead of implicitly unwrapped optional, simple check for URL can solve this problem.Just my 2 cents.

[kylef]

@tosbaha If Alamofire URL encoded your input, it still wouldn't become a URL because the : and / would be escaped.

http%3A%2F%2Fwww.example.com%2Fabc%2Fdef.asp%3Fk%3Dabc%20def

[tosbaha]

What I mean is, Alamofire could check if URL is valid instead of implicitly unwrapping it. Another way is encoding URL with something like escape function in ParameterEncoding.swift. I refactored all my code to use either parameters and when there are no parameters I use string extension to escape.

[cnoon]

@tosbaha the docs state:

public protocol URLStringConvertible {
    /**
        A URL that conforms to RFC 2396.

        Methods accepting a `URLStringConvertible` type parameter parse it according to RFCs 1738 and 1808.

        See https://tools.ietf.org/html/rfc2396
        See https://tools.ietf.org/html/rfc1738
        See https://tools.ietf.org/html/rfc1808
    */
    var URLString: String { get }
}

If you do not end up using the Alamofire ParameterEncoding, then you risk the possibility of certain characters not being escaped properly. This is exactly why we would recommend you always use the ParameterEncoding enumeration to do this. The URLStringConvertible protocol is merely convenience. It should not be used in cases where you are not confident that the URLString is valid.

We do not want to combine the concept of character escaping with the URLStringConvertible protocol because as @kylef mentioned, there can be some very strange behavior that causes way more harm then good.

Best of luck.

[tosbaha]

Thanks for the comment. I can understand that you don't want to increase complexity. However instead of implictly unwrapping we could check for correct URL and return error. I believe that failing is better than crashing. Best regards.