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

Adding a script with redirected @icon fails with error 500 #1331

Closed
monk-time opened this issue Feb 22, 2018 · 4 comments · Fixed by #1332
Closed

Adding a script with redirected @icon fails with error 500 #1331

monk-time opened this issue Feb 22, 2018 · 4 comments · Fixed by #1332
Labels
bug You've guessed it... this means a bug is reported. CODE Some other Code related issue and it should clearly describe what it is affecting in a comment.

Comments

@monk-time
Copy link

I have published a script that I can no longer update: apparently having // @icon http://www.imdb.com/favicon.ico prevents the site from accepting the script, although the same icon worked fine for the previous update a few months ago. I get this error from OpenUserJS:

500
@icon unsupported file type: undefined (file: undefined)

I suspect this is caused by OpenUserJS not expecting the target site to redirect the icon, in this case from http://www.imdb.com/favicon.ico to https://ia.media-imdb.com/images/G/01/imdb/images/favicon-2165806970.

@Martii
Copy link
Member

Martii commented Feb 23, 2018

Same issue as with my gravatar at https://gravatar.com/avatar/7ff58eb098c23feafa72e0b4cd13f396&r=G&s=48&default=identicon ... however that doesn't do a redirect.

If you would please create an issue with upstream I would appreciate it. I will tinker around with the Buffer to see if their example is incorrect when I get some free time. This will be helpful to see what they have to say about it.

Ref(s):

In the short term if you might try a data uri or hosting the image elsewhere without the MIME Content-Type of image/x-icon. e.g. convert it to a png.


Edit did a response test and got a 302. Will look into it... again when I have more than a few minutes. Thanks.

@Martii Martii added bug You've guessed it... this means a bug is reported. tracking upstream Waiting, watching, wanting. and removed tracking upstream Waiting, watching, wanting. labels Feb 23, 2018
@Martii Martii self-assigned this Feb 23, 2018
Martii added a commit to Martii/OpenUserJS.org that referenced this issue Feb 23, 2018
* Covers redirection automatically
* Fix undocumented type of `jpg` for `jpeg` *(my gravatar for example)*
* Left a response handler in there because **undoubtedly** there's going to be an issue cropping up with something in that arena
* Found an inline dependency to deal with later when more time available... added TODO
* `Buffer` method utilized is deprecated for base64 will affect repoManager... more TODO later

NOTE:
* I'm extremely short on time at the moment so thanks to everyone helping out with issues

Post OpenUserJS#1303 and should close OpenUserJS#1331
Martii added a commit that referenced this issue Feb 23, 2018
* Covers redirection automatically
* Fix undocumented type of `jpg` for `jpeg` *(my gravatar for example)*
* Left a response handler in there because **undoubtedly** there's going to be an issue cropping up with something in that arena
* Found an inline dependency to deal with later when more time available... added TODO
* `Buffer` method utilized is deprecated for base64 will affect repoManager... more TODO later

NOTE:
* I'm extremely short on time at the moment so thanks to everyone helping out with issues

Post #1303 and should close #1331

Auto-merge
@Martii Martii added the CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. label Feb 23, 2018
@Martii
Copy link
Member

Martii commented Feb 23, 2018

@monk-time
Should work for you now... thanks for the report.

@Martii Martii removed their assignment Feb 23, 2018
@thomas-ashcraft
Copy link

thomas-ashcraft commented Apr 15, 2018

500
`@icon` unsupported file type: undefined (file: undefined)

This I get when trying to import my userscript. I'd rather do nothing and look while webhook doin all updating job, but not this time. First I got pain head because of license forcing. And now that thing cannot accept icon. The same icon! I didnt changed it.

The only relevant link in Google is leading here.

Here is my metadata block:

// ==UserScript==
// @name         Alienware Arena helper
// @namespace    https://github.com/thomas-ashcraft
// @version      0.5.0
// @description  Earn daily ARP easily
// @author       Thomas Ashcraft
// @match        *://*.alienwarearena.com/*
// @match        *://*.alienwarearena.com//*
// @icon         https://www.alienwarearena.com/favicon.ico
// @license      GPL-2.0+; http://www.gnu.org/licenses/gpl-2.0.txt
// @grant        none
// @noframes
// ==/UserScript==

P.S. Was forced to remove metadata icon line completely to update script at least manually. Extremely frustrated.

@Martii
Copy link
Member

Martii commented Apr 16, 2018

First I got pain head because of license forcing.

Enforcement is the correct word... you agreed to it by continued use of the site in the TOS including not abusing any sites. It has been in there for years. End of this subject here as it's off topic and complain on your own time please.

https://www.alienwarearena.com/favicon.ico

It would appear that the compression isn't supported on the .ico e.g. they use an unsupported frame and in a few browsers tested as well. Even has a warning in my .ico editor about it.

Extremely frustrated.

Your choices for survival are:

  1. Use a data URI of the size up to the maximum mentioned previously in another issue (currently 256x256)... I would suggest something more reasonable in size and fixed not a set of fixed images.
  2. Host it somewhere else without that frame
  3. Use a different, preferably more compatible, icon image. You could use Google S2 at https://www.google.com/s2/favicons?domain=www.alienwarearena.com
  4. Open an issue with that dependency... not us.

Our choices are migrating towards:

  1. Remove .ico support (which I'm still contemplating) ... or
  2. attempt to limit it's file size as well (with a reject on server hosting error with lack of size information sent) because uncompressed that icon is ~1.5MiB (compressed it is still ~359.1KiB which seems excessive at first glance... would need to compare some still images instead of a set of images) which is again needlessly wasting everyone's bandwidth which is why the check is put in. e.g. as I put elsewhere one doesn't need to be a jerk to your users and especially the visitors. It's bad practice and bad showmanship which can lead to down voting and removal of a script if it gets too many/or any complaints.
  3. If and when CORS is implemented that will also limit @icon usage depending on the target servers properties.

Refs:

@OpenUserJS OpenUserJS locked as resolved and limited conversation to collaborators Apr 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug You've guessed it... this means a bug is reported. CODE Some other Code related issue and it should clearly describe what it is affecting in a comment.
Development

Successfully merging a pull request may close this issue.

3 participants