Objectmapper: URLTransform - handle URLs with spaces

[rodrigo-lima]

I was testing a case where one of the JSON attributes is an URL string containing spaces.
For example:

{
    "contentId": 371923734, 
    "contentURL": "http://someserver.com/content/abcd/Vision Corporate Responsibility.html"
}

My mapping struct is something like this:

struct ContentResource: Mappable {
    init?(_ map: ObjectMapper.Map) { }

    var contentId:Int
    var contentURL:NSURL?

    mutating func mapping(map: Map) {
        contentId  <- map["contentId"],URLTransform()
        contentURL <- (map["contentURL"],URLTransform())
    }
}

After debugging for a while, it turns out that the result of contentURL <- map response is nil, because NSURL(string: "url with spaces") returns nil.
We need to encode the URL string before calling _NSURL_. (see http://stackoverflow.com/a/24879794 )

The fix below seems to work for me, though there's some more discussion on the SO thread above on how to handle other chars ( + , & , etc.. ), for a more robust/complete fix

URLTransform.swift

public class URLTransform: TransformType {
    public typealias Object = NSURL
    public typealias JSON = String

    public init() {}

    public func transformFromJSON(value: AnyObject?) -> NSURL? {
        if let URLString = value as? String,
            // handle URLs with spaces (and other non-escaped chars)
            escapedURL = URLString.stringByAddingPercentEncodingWithAllowedCharacters(NSCharacterSet.URLQueryAllowedCharacterSet()) {
            return NSURL(string: escapedURL)
        }
        return nil
    }

    public func transformToJSON(value: NSURL?) -> String? {
        if let URL = value {
            return URL.absoluteString
        }
        return nil
    }
}

[tristanhimmelman]

Thanks for the fix. I updated the transform

[rodrigo-lima]

Hi thanks for looking into this!
My bad that I didn't update this with a slightly better version (coincidentally, I just found the bug this morning....)

Sometimes, the server JSON returns:

• https://blah.com/somepath?fileName=abce space -- this requires escaping
• https://blah.com/somepath?someDate=2016-07-08T02%3A19%3A25.113-0700 -- this is good, already escaped on the server side

you might end up with double encoded %, if server returns something like

• https://blah.com/somepath?fileName=abce space&someDate=2016-07-08T02%3A19%3A25.113-0700
  In this case, I'd file the bug against the server :)

This is the code I have now, but I still need to run some more tests to make sure all cases are covered.
If you prefer, I could submit a pull-request after adding some more Unit Tests.

    public func transformFromJSON(value: AnyObject?) -> NSURL? {
        if let URLString = value as? String {
            if let url = NSURL(string: URLString) {
                return url
            }
            else if let escapedString = URLString.stringByAddingPercentEncodingWithAllowedCharacters(NSCharacterSet.URLQueryAllowedCharacterSet()) {
                return NSURL(string: escapedString)
            }
        }
        return nil
    }

[tristanhimmelman]

A pull request with this update and some unit tests would be great! I appreciate the help 👍

[aydegee]

I also experienced an issue with the encoding introduced in 9b8163a and created this PR which makes the encoding optional: https://github.com/Hearst-DD/ObjectMapper/pull/566

Not sure if I should open a new issue or continue the discussion here? but I wondered if the default behaviour _should_ be to encode the URL first or leave that up to the caller? As noted in this issue and comments on 9b8163a it's difficult for ObjectMapper to know the context of the URL it is transforming in order to make the correct decisions about encoding. For example the string might already be encoded.

As it stands it's not possible to take an NSURL which contains spaces, map it to a string and back to an NSURL and end up with the same value you started with because it gets double-encoded.

Having used RestKit previously I recall it opted to avoid any encoding and just passed the string directly to the NSURL initialiser. I also noticed the Apple docs state that stringByAddingPercentEncodingWithAllowedCharacters, which is used to do the encoding, is not suitable to encode entire URLs but just components of URLs. Perhaps ObjectMapper is taking on too much responsibility in this regard?

[MasterCarl]

Having just spent a fair amount of time debugging failing Amazon S3 requests, I think that shouldEncodeURLString should default to false. The rationale behind this is that trying to transform Strings with spaces will fail, while double-encoding URLs results in valid yet unusable URLs. However, introducing this now would be a breaking change.

[iliaskarim]

Seconding @MasterCarl and @aydegee's comments: shouldEncodeURLString ought to default to false because ObjectMapper should assume that the payloads it parses--including the contents of URL strings--are well-formed.

The current workaround that has been implemented is a _regression_ and reason enough for a breaking update with the next minor version. Currently, a URL like https://itmeo.com/public/webgradients_png/030%20Happy%20Fisher.png is parsed into a broken URL: https://itmeo.com/public/webgradients_png/030%2520Happy%2520Fisher.png 😞

[tristanhimmelman]

I have merged #900 to fix this. Thanks