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

Some favicons cannot be loaded / saved. #10120

Closed
memo-567 opened this issue Feb 26, 2023 · 11 comments · Fixed by #10186
Closed

Some favicons cannot be loaded / saved. #10120

memo-567 opened this issue Feb 26, 2023 · 11 comments · Fixed by #10186
Labels
⚠️bug? This might be a bug

Comments

@memo-567
Copy link

💡 Summary

Some favicons cannot be loaded / saved.

🥰 Expected Behavior

Favicons should be saved and displayed.

🤬 Actual Behavior

mk hw2k com -  2023 - 02 - 26  14 - 35

📝 Steps to Reproduce

Logfile:

INFO *	[download]	Downloading https://chaos.social/packs/media/icons/android-chrome-36x36-4c61fdb42936428af85afdbf8c6a45a8.png to /tmp/tmp-1319-3XHancQuPy53 ...
DONE *	[download]	Download finished: https://chaos.social/packs/media/icons/android-chrome-36x36-4c61fdb42936428af85afdbf8c6a45a8.png
ERR  *	[server]	StatusError: Unexpected mime
INFO *	[download]	Downloading https://chaos.social/packs/media/icons/favicon-48x48-c1197e9664ee6476d2715a1c4293bf61.png to /tmp/tmp-1319-GShuvPKd9p70 ...
DONE *	[download]	Download finished: https://chaos.social/packs/media/icons/favicon-48x48-c1197e9664ee6476d2715a1c4293bf61.png
ERR  *	[server]	StatusError: Unexpected mime
INFO *	[metadata]	Fetching metadata of chaos.social ...
INFO *	[metadata]	Fetching nodeinfo of chaos.social ...
INFO *	[metadata]	Fetching HTML of chaos.social ...
DONE *	[metadata]	Successfuly fetched nodeinfo of chaos.social
DONE *	[metadata]	Successfuly fetched metadata of chaos.social
DONE *	[metadata]	Successfuly updated metadata of chaos.social

📌 Environment

Misskey versions: 13.7.5 / 13.8.0 / 13.8.1
Your OS: Debian 11
Your browser: all

@memo-567 memo-567 added the ⚠️bug? This might be a bug label Feb 26, 2023
@saschanaz
Copy link
Member

sharpがICOはサポートしていないせいだと聞きました / I heard this is because sharp does not support ICO format.

@memo-567
Copy link
Author

I don't know what this sharp is, but is there a fix for it?

@saschanaz
Copy link
Member

ここから発生していますね / It's coming from here:

const isConvertibleImage = isMimeImage(file.mime, 'sharp-convertible-image');
const isAnimationConvertibleImage = isMimeImage(file.mime, 'sharp-animation-convertible-image');
if (
'emoji' in request.query ||
'avatar' in request.query ||
'static' in request.query ||
'preview' in request.query ||
'badge' in request.query
) {
if (!isConvertibleImage) {
// 画像でないなら404でお茶を濁す
throw new StatusError('Unexpected mime', 404);
}
}

多分ICOファイルはそのまま使っていいかもです。/ I guess it's probably fine to skip conversion for ICO files 🤔

@tamaina
Copy link
Contributor

tamaina commented Feb 26, 2023

icoファイルは大変大きいので、クライアントに読ませるのは避けたい

@acid-chicken
Copy link
Member

ico 自体のデコードは楽なはず (e.g. https://github.com/image-rs/image/blob/master/src/codecs/ico/decoder.rs) なのでなんとかなってほしい

@memo-567
Copy link
Author

This seems mostly affect custom favicons.

As long as it doesn't work properly, I've switched it off for now.

@tamaina
Copy link
Contributor

tamaina commented Mar 1, 2023

インスタンスのアイコンにこんなにico突っ込まれてたっけ?

@tamaina
Copy link
Contributor

tamaina commented Mar 1, 2023

ico 自体のデコードは楽なはず

実装が簡単とは言わなかったな?

@memo-567
Copy link
Author

memo-567 commented Mar 1, 2023

Yes, it should usually display as shown in the screenshot.

mk hw2k com -  2023 - 03 - 01  12 - 24

It just doesn't seem to work with custom favicons.
Not even with my Misskey instance's own custom favicon.

@tamaina
Copy link
Contributor

tamaina commented Mar 3, 2023

media-proxy v0.0.17で対応した

@tamaina
Copy link
Contributor

tamaina commented Mar 3, 2023

本体メディアプロキシはまだ

syuilo added a commit that referenced this issue Mar 4, 2023
* enhance(server): downloadUrlでContent-Dispositionからファイル名を取得
Resolve #10036
Resolve #4750

* untitled

* オブジェクトストレージのContent-Dispositionのファイル名の拡張子をContent-Typeに添ったものにする

* ✌️

* tiff

* fix filename

* add test

* /files/でもContent-Disposition

* enhance(server): メディアプロキシでico,bmpを読めるように
Fix #10120

* comment

* fix test

---------

Co-authored-by: syuilo <Syuilotan@yahoo.co.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️bug? This might be a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants