Square brackets in remote_x_url not being handled correctly

[kevinansfield]

When attempting to assign a url using the Model.remote_file_url accessor most characters are encoded properly however square brackets are left as they are which generates a URI::InvalidURIError exception:

bad URI(is not URI?): http://my-bucket.s3.amazonaws.com/uploads/PopChop%20-%20Cut%20The%20Fuck%20Up%20[The%20Remix%20Project]%20-Special%20Edition%20-%2003%20-%20The%20Terminator%20Interlude.mp3
/Users/kev/.rvm/rubies/ruby-1.9.2-p0/lib/ruby/1.9.1/uri/common.rb:156:in `split'
/Users/kev/.rvm/rubies/ruby-1.9.2-p0/lib/ruby/1.9.1/uri/common.rb:174:in `parse'
/Users/kev/.rvm/rubies/ruby-1.9.2-p0/lib/ruby/1.9.1/uri/common.rb:628:in `parse'
/Users/kev/.rvm/gems/ruby-1.9.2-p0@global/gems/carrierwave-0.5.1/lib/carrierwave/uploader/download.rb:16:in `initialize'
/Users/kev/.rvm/gems/ruby-1.9.2-p0@global/gems/carrierwave-0.5.1/lib/carrierwave/uploader/download.rb:55:in `new'
/Users/kev/.rvm/gems/ruby-1.9.2-p0@global/gems/carrierwave-0.5.1/lib/carrierwave/uploader/download.rb:55:in `download!'
/Users/kev/.rvm/gems/ruby-1.9.2-p0@global/gems/carrierwave-0.5.1/lib/carrierwave/mount.rb:314:in `remote_url='
/Users/kev/.rvm/gems/ruby-1.9.2-p0@global/gems/carrierwave-0.5.1/lib/carrierwave/mount.rb:194:in `remote_mp3_url='
/Users/kev/code/rails/xx/app/jobs/process_song.rb:13:in `perform'

There is a ruby bug report at http://redmine.ruby-lang.org/issues/show/3457 suggesting that URI.encode should not be used for this purpose.

I have attempted encoding the square brackets directly before handing it off to carrierwave but this appears to end up with double encoding of the % characters.

[trevorturk]

If you have time, please investigate a solution. Do they suggest something else as an alternative to URI.encode?

[trevorturk]

Closing in favor of: https://github.com/jnicklas/carrierwave/issues#issue/199

I'd like carrierwave to provide an easy way for people to override/improve the url parsing where necessary.

[ngauthier]

I'd like to reopen this issue.

IMO, carrierwave should not be responsible for escaping and unescaping the url given to it. It should pass it directly to Net::HTTP.

If the user is loading urls from a potentially unsafe source, they should choose to escape them. In my case, I am loading urls from a csv from a trusted source. The odd thing here is this:

Parses with % escaped values fine:

1.9.3p0 :006 > URI.parse("http://www.taosgifts.co.uk/ekmps/shops/taos/images/money-to-burn-novelty-candles-5-10-20-notes-set-of-3--%5B2%5D-21475-p.jpg")
 => #<URI::HTTP:0x00000000fc8370 URL:http://www.taosgifts.co.uk/ekmps/shops/taos/images/money-to-burn-novelty-candles-5-10-20-notes-set-of-3--%5B2%5D-21475-p.jpg>

If I escape already escaped content, it's fine:

1.9.3p0 :011 > URI.escape("http://www.taosgifts.co.uk/ekmps/shops/taos/images/money-to-burn-novelty-candles-5-10-20-notes-set-of-3--%5B2%5D-21475-p.jpg")
 => "http://www.taosgifts.co.uk/ekmps/shops/taos/images/money-to-burn-novelty-candles-5-10-20-notes-set-of-3--%255B2%255D-21475-p.jpg" 
1.9.3p0 :010 > URI.parse(URI.escape("http://www.taosgifts.co.uk/ekmps/shops/taos/images/money-to-burn-novelty-candles-5-10-20-notes-set-of-3--%5B2%5D-21475-p.jpg"))
 => #<URI::HTTP:0x00000000c19e68 URL:http://www.taosgifts.co.uk/ekmps/shops/taos/images/money-to-burn-novelty-candles-5-10-20-notes-set-of-3--%255B2%255D-21475-p.jpg> 

If you unescape it then escape it, URI.escape doesn't escape the [] and it crashes:

1.9.3p0 :012 > URI.escape(URI.unescape("http://www.taosgifts.co.uk/ekmps/shops/taos/images/money-to-burn-novelty-candles-5-10-20-notes-set-of-3--[2]-21475-p.jpg"))
 => "http://www.taosgifts.co.uk/ekmps/shops/taos/images/money-to-burn-novelty-candles-5-10-20-notes-set-of-3--[2]-21475-p.jpg" 
1.9.3p0 :013 > URI.parse(URI.escape(URI.unescape("http://www.taosgifts.co.uk/ekmps/shops/taos/images/money-to-burn-novelty-candles-5-10-20-notes-set-of-3--[2]-21475-p.jpg")))
URI::InvalidURIError: bad URI(is not URI?): http://www.taosgifts.co.uk/ekmps/shops/taos/images/money-to-burn-novelty-candles-5-10-20-notes-set-of-3--[2]-21475-p.jpg
    from /home/nick/.rvm/rubies/ruby-1.9.3-p0-falcon/lib/ruby/1.9.1/uri/common.rb:176:in `split'
    from /home/nick/.rvm/rubies/ruby-1.9.3-p0-falcon/lib/ruby/1.9.1/uri/common.rb:211:in `parse'
    from /home/nick/.rvm/rubies/ruby-1.9.3-p0-falcon/lib/ruby/1.9.1/uri/common.rb:747:in `parse'
    from (irb):13
    from /home/nick/.rvm/rubies/ruby-1.9.3-p0-falcon/bin/irb:16:in `<main>'

This is a known bug in ruby but has been rejected for resolution. I propose either:

1. Add a preserve_remote_uris option that defaults to false, which, when enabled, does not unescape+escape uris
2. Remove unescape+escape and rely on the calling code to sanitize when appropriate

I'm going to do #2 right now and submit a pull request. We can continue there.