IMGKit.new fails on url' with ampersand

[pacMakaveli]

Hello,

So this is a weird one and one that gave me a couple of headaches.
Why is IMGKit failing when given url's contain ampersands(&) ?

I know I can 'sanitize' the URL to remove the ampersand. However, I don't want to do it, really.

Here's the log.

Failed example:

irb(main):115:0> Bookmark.find(2).test
  Bookmark Load (0.5ms)  SELECT  `bookmarks`.* FROM `bookmarks` WHERE `bookmarks`.`id` = 2 LIMIT 1
"http://www.sitepoint.com/recreating-google-images-search-layout-css/?utm_source=html5weekly&utm_medium=email"
=> #<File:tmp/screens/bookmarks/capture_220150214-4859-zx7b06.png (closed)>
irb(main):116:0>

Working example:

irb(main):117:0> Bookmark.find(2).test
  Bookmark Load (0.4ms)  SELECT  `bookmarks`.* FROM `bookmarks` WHERE `bookmarks`.`id` = 2 LIMIT 1
"http://www.sitepoint.com/recreating-google-images-search-layout-css/"
=> #<File:tmp/screens/bookmarks/capture_220150214-4859-sj0p2h.png (closed)>
irb(main):118:0>

[Ricardonacif]

Man I'm facing the same issue. Did you find any solution?

[pacMakaveli]

@Ricardonacif

I mentioned this in the post.. It's not ideal, but it works.

I know I can 'sanitize' the URL to remove the ampersand. However, I don't want to do it, really.

html  = b.url.split('?')[0]

I'm sure there's a better way, it's something I've done back in february, if I were to approach this again I'd do it different.

[Ricardonacif]

@pacMakaveli I don't see a problem on doing that. In fact, that's what pdfkit gem did.

[pacMakaveli]

@Ricardonacif

I don't see a valid reason why IMGKit should fail because of that.
Here's a reason why removing the ? or & from the url is a bad idea:

http://www.argos.co.uk/webapp/wcs/stores/servlet/Browse?s=Relevance&storeId=10151&langId=110&catalogId=25051&mRR=true&c_1=1%7Ccategory_root%7CTechnology%7C33006169&c_2=2%7C33006169%7CLaptops%20and%20PCs%7C33007795&c_3=3%7Ccat_33007795%7CLaptops%20and%20netbooks%7C33014243&r_001=1%7C3DTV%20Glasses-Brands%7CApple%7C1&r_002=1%7CBrands%7CApple%7C1&r_003=1%7CLaptops%20%26%20Netbooks-Screen%20size%20range%20(in)%7C12%20-%2013.9in%7C1&r_004=1%7CDesktop%20Computers%20%26%20All%20In%20Ones-RAM%20(GB)%7C8%7C1

Removing ? : http://www.argos.co.uk/webapp/wcs/stores/servlet/Browse -> 404
Removing & , in some cases, will give a 404 again..

It really depends on your use of IMGKit.. Where I use it, escaping the URL becomes a pain and sometimes fails.

[drewB]

@Ricardonacif your pull request is exact what is needed. Hopefully will be merged in soon.