Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.ICO file resize throws 500 Internal Server Error #52

Closed
ashish-soni opened this issue Aug 1, 2018 · 7 comments
Closed

.ICO file resize throws 500 Internal Server Error #52

ashish-soni opened this issue Aug 1, 2018 · 7 comments

Comments

@ashish-soni
Copy link

ashish-soni commented Aug 1, 2018

Resize of .ICO files are throwing http error 500, because sharp does not support it and gm also support only read operation of .ico file.

http://saladelana.nl/____impro/1/onewebmedia/faviconsal.ico?etag=%2247e-587d003b%22&sourceContentType=image%2Fvnd.microsoft.icon&resize=16,16&format=ico

Is it possible to use gm module to convert .ico to .png and perfrom resize operation?

I tried but the issue is that sourceContentType given in querystring is ignored and goes inside leftOverQueryStringFragments and getMockFileNameForContentType returns undefined.

Operations:
[ { sourceContentType: undefined,
name: 'png',
args: [],
usedQueryStringFragment: 'png' },
{ sourceContentType: 'image/png',
name: 'resize',
args: [ 12, 12 ]

If code can make first operation's sourceContentType to image/vnd.microsoft.icon then it works with below url.

http://saladelana.nl/____impro/1/onewebmedia/faviconsal.ico?gm&png&etag=%2247e-587d003b%22&sourceContentType=image%2Fvnd.microsoft.icon&resize=16,16&format=ico

Is this way correct or am I missing something?

@ashish-soni ashish-soni changed the title ICO file resize throws 500 Internal Server Error .ICO file resize throws 500 Internal Server Error Aug 1, 2018
@papandreou
Copy link
Owner

Hey Ashish, long time no see!

Yeah, this is a missing feature. It was also a problem in another product that uses this module :). I think there's even an issue about it in a tracker that I no longer have access to, maybe Asana somewhere under "Impro"? Would be helpful if you could find that and copy the information it in here? I'm fairly sure I did some initial research.

@ashish-soni
Copy link
Author

I am good, how about you?
Sure, I will try to find info in asana and put it here.

@ashish-soni
Copy link
Author

0001-Fix-for-.ICO-file-resize-throwing-500-error.patch.txt

Hi @papandreou

I have attached patch for this fix. It looks safe to me.

If you think, it is safe to merge, can you please do the needful?

@papandreou
Copy link
Owner

Any chance you could include a test case? It'll also be easier to manage if you open a pull request so CI will run :)

@ashish-soni
Copy link
Author

Is the patch looks ok to you? I will do the rest. Just let me know :)

@ashish-onecom
Copy link
Contributor

I have added pull request and test too.

@gustavnikolaj
Copy link
Collaborator

The fix to the bug mentioned here was merged and released in 8.0.1 after #53 being merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants