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

[Bug]: Images not being rendered properly after 0.19.6 upgrade #5196

Open
4 of 5 tasks
DraconicNEO opened this issue Nov 12, 2024 · 20 comments
Open
4 of 5 tasks

[Bug]: Images not being rendered properly after 0.19.6 upgrade #5196

DraconicNEO opened this issue Nov 12, 2024 · 20 comments
Labels
bug Something isn't working

Comments

@DraconicNEO
Copy link

DraconicNEO commented Nov 12, 2024

Requirements

  • Is this a bug report? For questions or discussions use https://lemmy.ml/c/lemmy_support
  • Did you check to see if this issue already exists?
  • Is this only a single bug? Do not put multiple bugs in one issue.
  • Do you agree to follow the rules in our Code of Conduct?
  • Is this a backend issue? Use the lemmy-ui repo for UI / frontend issues.

Summary

After the recent upgrade to 0.19.6 on my Home instance it appears certain image posts are no longer rendering the image and instead are treating it as a link, very annoying bug for sure.

Steps to Reproduce

  1. Find certain posts with embedded external images
  2. Images will not render in the post and will instead appear as a link to the image file

Technical Details

Not an Admin, can't provide logs.

Here's a screenshot though:
image

What it should look like:
image

Version

BE 0.19.6

Lemmy Instance URL

lemmy.dbzer0.com

@DraconicNEO DraconicNEO added the bug Something isn't working label Nov 12, 2024
@DraconicNEO
Copy link
Author

@db0 Mentioning you in case you might be able to provide information on this image related bug that I can't.

@db0
Copy link
Contributor

db0 commented Nov 12, 2024

Can you also provide the relevant link?

@Nothing4You
Copy link
Collaborator

Nothing4You commented Nov 13, 2024

this is probably at least partially the same issue that was reported in LemmyNet/lemmy-ui#2805 already when it comes to thumbnail generation.

the post in the screenshots is https://lemmy.dbzer0.com/post/31254852.

looking at the API in this case, https://lemmy.dbzer0.com/api/v3/post?id=31254852, it shows the content type as binary/octet-stream.
the server hosting https://image.civitai.com/xG1nkqKTMzGDvpLrqFT7WA/a5e8dddc-9962-46ce-9f9e-bc0933ce91b7/original=true,quality=90,optimized=true/00091-28-1440-864-2024-11-02-0.7.jpeg does not return an appropriate image content type.
you'll also notice when you open the url this points to it'll download the file instead of displaying it in the browser.

as the response is neither html (which could reference an image in metadata) nor an image format in the content type this leads to no thumbnail generation.
lemmy-ui then checks whether the post has a thumbnail url and fails when there is no thumbnail url on the post, defaulting to a regular link post, even if the simple image url check passes.

related code:

/// Generates and saves a post thumbnail and metadata.
///
/// Takes a callback to generate a send activity task, so that post can be federated with metadata.
///
/// TODO: `federated_thumbnail` param can be removed once we federate full metadata and can
/// write it to db directly, without calling this function.
/// https://github.com/LemmyNet/lemmy/issues/4598
pub async fn generate_post_link_metadata(
post: Post,
custom_thumbnail: Option<Url>,
send_activity: impl FnOnce(Post) -> Option<SendActivityData> + Send + 'static,
local_site: Option<LocalSite>,
context: Data<LemmyContext>,
) -> LemmyResult<()> {
let metadata = match &post.url {
Some(url) => fetch_link_metadata(url, &context).await.unwrap_or_default(),
_ => Default::default(),
};
let is_image_post = metadata
.content_type
.as_ref()
.is_some_and(|content_type| content_type.starts_with("image"));
// Decide if we are allowed to generate local thumbnail
let allow_sensitive = local_site_opt_to_sensitive(&local_site);
let allow_generate_thumbnail = allow_sensitive || !post.nsfw;
let image_url = if is_image_post {
post.url
} else {
metadata.opengraph_data.image.clone()
};
let thumbnail_url = if let (false, Some(url)) = (is_image_post, custom_thumbnail) {
proxy_image_link(url, &context).await.ok()
} else if let (true, Some(url)) = (allow_generate_thumbnail, image_url) {
generate_pictrs_thumbnail(&url, &context)
.await
.ok()
.map(Into::into)
} else {
metadata.opengraph_data.image.clone()
};
let form = PostUpdateForm {
embed_title: Some(metadata.opengraph_data.title),
embed_description: Some(metadata.opengraph_data.description),
embed_video_url: Some(metadata.opengraph_data.embed_video_url),
thumbnail_url: Some(thumbnail_url),
url_content_type: Some(metadata.content_type),
..Default::default()
};
let updated_post = Post::update(&mut context.pool(), post.id, &form).await?;
if let Some(send_activity) = send_activity(updated_post) {
ActivityChannel::submit_activity(send_activity, &context).await?;
}
Ok(())
}

https://github.com/LemmyNet/lemmy-ui/blob/8d95373cf889f2d2b5342525c65f96f68c92e0df/src/shared/components/post/post-listing.tsx#L327

@DraconicNEO
Copy link
Author

DraconicNEO commented Nov 13, 2024

Can you also provide the relevant link?

Sure here you go:
https://lemmy.dbzer0.com/post/31254852

I also just found this one while browsing [email protected]:
https://lemmy.dbzer0.com/post/31415696

Seems to be completely random which posts it happens to.

@DraconicNEO
Copy link
Author

this is probably at least partially the same issue that was reported in LemmyNet/lemmy-ui#2805 already when it comes to thumbnail generation.

the post in the screenshots is https://lemmy.dbzer0.com/post/31254852.

looking at the API in this case, https://lemmy.dbzer0.com/api/v3/post?id=31254852, it shows the content type as binary/octet-stream. the server hosting https://image.civitai.com/xG1nkqKTMzGDvpLrqFT7WA/a5e8dddc-9962-46ce-9f9e-bc0933ce91b7/original=true,quality=90,optimized=true/00091-28-1440-864-2024-11-02-0.7.jpeg does not return an appropriate image content type. you'll also notice when you open the url this points to it'll download the file instead of displaying it in the browser.

as the response is neither html (which could reference an image in metadata) nor an image format in the content type this leads to no thumbnail generation. lemmy-ui then checks whether the post has a thumbnail url and fails when there is no thumbnail url on the post, defaulting to a regular link post, even if the simple image url check passes.

related code:

/// Generates and saves a post thumbnail and metadata.
///
/// Takes a callback to generate a send activity task, so that post can be federated with metadata.
///
/// TODO: `federated_thumbnail` param can be removed once we federate full metadata and can
/// write it to db directly, without calling this function.
/// https://github.com/LemmyNet/lemmy/issues/4598
pub async fn generate_post_link_metadata(
post: Post,
custom_thumbnail: Option<Url>,
send_activity: impl FnOnce(Post) -> Option<SendActivityData> + Send + 'static,
local_site: Option<LocalSite>,
context: Data<LemmyContext>,
) -> LemmyResult<()> {
let metadata = match &post.url {
Some(url) => fetch_link_metadata(url, &context).await.unwrap_or_default(),
_ => Default::default(),
};
let is_image_post = metadata
.content_type
.as_ref()
.is_some_and(|content_type| content_type.starts_with("image"));
// Decide if we are allowed to generate local thumbnail
let allow_sensitive = local_site_opt_to_sensitive(&local_site);
let allow_generate_thumbnail = allow_sensitive || !post.nsfw;
let image_url = if is_image_post {
post.url
} else {
metadata.opengraph_data.image.clone()
};
let thumbnail_url = if let (false, Some(url)) = (is_image_post, custom_thumbnail) {
proxy_image_link(url, &context).await.ok()
} else if let (true, Some(url)) = (allow_generate_thumbnail, image_url) {
generate_pictrs_thumbnail(&url, &context)
.await
.ok()
.map(Into::into)
} else {
metadata.opengraph_data.image.clone()
};
let form = PostUpdateForm {
embed_title: Some(metadata.opengraph_data.title),
embed_description: Some(metadata.opengraph_data.description),
embed_video_url: Some(metadata.opengraph_data.embed_video_url),
thumbnail_url: Some(thumbnail_url),
url_content_type: Some(metadata.content_type),
..Default::default()
};
let updated_post = Post::update(&mut context.pool(), post.id, &form).await?;
if let Some(send_activity) = send_activity(updated_post) {
ActivityChannel::submit_activity(send_activity, &context).await?;
}
Ok(())
}

https://github.com/LemmyNet/lemmy-ui/blob/8d95373cf889f2d2b5342525c65f96f68c92e0df/src/shared/components/post/post-listing.tsx#L327

I wasn't sure if they are the same one, since that one seemed like it was for link posts with a thumbnail, but this is for image posts which aren't being detected as images at all.

@Nutomic
Copy link
Member

Nutomic commented Nov 13, 2024

Agree with @Nothing4You, seems there is nothing we can do (other than contacting the image hoster about adding proper content headers).

I also just found this one while browsing [email protected]:
https://lemmy.dbzer0.com/post/31415696

This one works fine on a test server, so most likely there was a network error when the thumbnail was generated.

@db0
Copy link
Contributor

db0 commented Nov 13, 2024

Civitai is open sourced on github, so someone could open an issue about it?

@r-hmn
Copy link

r-hmn commented Nov 13, 2024

@Nothing4You thanks for mentioning. The issue LemmyNet/lemmy-ui#2805 happens for every newly created post, either via api or via gui. i hope it can be moved to this repository

@DraconicNEO
Copy link
Author

Civitai is open sourced on github, so someone could open an issue about it?

But it also seems to be happening with other Lemmy instances since the second post URL was a pict-rs link from Lemmy.world

@DraconicNEO
Copy link
Author

Question, do you guys think that the effects of this bug will persist after the fix, or do you think it'll be retroactively fixed? I hope it's the latter otherwise a lot of content is going to be messed up by this.

@db0 I don't think we really need to report this to civitai, this is a Lemmy problem, Lemmy is deciding images are links and not images, and Lemmy shouldn't be doing this.

The change that caused this in 0.19.6 is wrong, it shouldn't be like this, if a post ends as an image it should attempt to render as an image. All the instances which haven't upgraded don't have this issue, even on posts with the affected civitai links.

@Nothing4You
Copy link
Collaborator

do you guys think that the effects of this bug will persist after the fix, or do you think it'll be retroactively fixed?

thumbnails that are never generated will not be created later on. posts do not get modified for metadata after initial creation and metadata generation.

I don't think we really need to report this to civitai, this is a Lemmy problem, Lemmy is deciding images are links and not images, and Lemmy shouldn't be doing this.

if civitai claims images are just random binary (via content type header) this is up to civitai to fix. in this specific case the url ends with an image type file suffix, but that doesn't mean much on the internet.

lemmy could probably add some logic for mime type sniffing as fallback, but websites should be expected to provide an appropriate content type in the response.

@Nutomic
Copy link
Member

Nutomic commented Nov 15, 2024

The change that caused this in 0.19.6 is wrong, it shouldn't be like this, if a post ends as an image it should attempt to render as an image. All the instances which haven't upgraded don't have this issue, even on posts with the affected civitai links.

Ive tested the civitai image with 0.19.5 and it also doesnt generate a thumbnail. However lemmy-ui does load the full image with 864 × 1440 px and displays it as thumbnail, which means its related to LemmyNet/lemmy-ui#2797. Anyway using such a huge image as thumbnail is a waste of resources so it makes no sense.

@DraconicNEO
Copy link
Author

DraconicNEO commented Nov 16, 2024

do you guys think that the effects of this bug will persist after the fix, or do you think it'll be retroactively fixed?

thumbnails that are never generated will not be created later on. posts do not get modified for metadata after initial creation and metadata generation.

I don't think we really need to report this to civitai, this is a Lemmy problem, Lemmy is deciding images are links and not images, and Lemmy shouldn't be doing this.

if civitai claims images are just random binary (via content type header) this is up to civitai to fix. in this specific case the url ends with an image type file suffix, but that doesn't mean much on the internet.

Not in agreement, if content ends with an image file suffix it's almost always intended to be an image. I've never seen content that wasn't an image distributed as .png, .jpg, .bmp, etc. and if that did happen and was posted to Lemmy for whatever reason, it just wouldn't render a usable image.
On the flip side, if content isn't being distributed as a valid image suffix it probably wasn't intended to be an image. This system works almost flawlessly and doesn't have weird annoying bugs like randomly broken posts that display a URL when they should show an image.

lemmy could probably add some logic for mime type sniffing as fallback, but websites should be expected to provide an appropriate content type in the response.

I'm more talking about all the Lemmy image posts like these which are just randomly broken after the update. I'm assuming that by what you're saying that those probably will not be fixed in the update. Since it's affecting more than just civitai images it's more than just a Civitai problem.

@r-hmn
Copy link

r-hmn commented Nov 16, 2024

All posts with a youtube link created with v0.19.6 & v0.19.7 have no thumbnail ( LemmyNet/lemmy-ui#2805 )

@flamingo-cant-draw
Copy link
Contributor

I'm more talking about all the Lemmy image posts like these which are just randomly broken after the update. I'm assuming that by what you're saying that those probably will not be fixed in the update. Since it's affecting more than just civitai images it's more than just a Civitai problem.

That 196 post seems unrelated to the Civitai problem and genuinely a Lemmy issue. I took a look at that post on some > 0.19.5 instances and it's very hit and miss

Works on:
Slrpnk
discuss.tchncs.de
Sopuli
infosec.pub

Doesn't work on:
discuss.online
lemmy.zip
dbzer0
midwest.social
aussie.zone

In fact, both aussie.zone and midwest.social have a lot of broken image posts. The ones with the broken posts don't have a thumnail_url in the API response, which is a regression as that same post on feddit.uk and programming.dev doesn't have a thumnail_url and lemmy-ui still produces an image post.

@dessalines
Copy link
Member

@DraconicNEO read our code of conduct and please do not downthumb people for trying to help answer questions.

@DraconicNEO
Copy link
Author

@DraconicNEO read our code of conduct and please do not downthumb people for trying to help answer questions.

I downthumbed because I thought the suggestion seemed unhelpful, but I apologize if I made a mistake.

@DraconicNEO
Copy link
Author

DraconicNEO commented Nov 18, 2024

I'm more talking about all the Lemmy image posts like these which are just randomly broken after the update. I'm assuming that by what you're saying that those probably will not be fixed in the update. Since it's affecting more than just civitai images it's more than just a Civitai problem.

That 196 post seems unrelated to the Civitai problem and genuinely a Lemmy issue. I took a look at that post on some > 0.19.5 instances and it's very hit and miss

Works on: Slrpnk discuss.tchncs.de Sopuli infosec.pub

Doesn't work on: discuss.online lemmy.zip dbzer0 midwest.social aussie.zone

In fact, both aussie.zone and midwest.social have a lot of broken image posts. The ones with the broken posts don't have a thumnail_url in the API response, which is a regression as that same post on feddit.uk and programming.dev doesn't have a thumnail_url and lemmy-ui still produces an image post.

Honestly it seems like whatever change was made to change whether posts are deemed as images or not, seems like a case of fixing what isn't broken and inadvertently breaking what was already working before without having to do anything extra.
Your examples seem to be working now but there are now others which aren't, even on servers where they seem to be working:

Kind of feel like we should maybe have a fallback to the old way of doing it, or allow image posts to be manually declared as such with a warning if the image request data isn't present.
The way I see it false positives declared as images which cannot be rendered are better than false negatives where the image is there but isn't recognized screwing up the post for people on the instance. Screw ideology about how things should be, shoulds are worthless when we can't change how things are.

To put it better, since many people here are likely visual learners.

This:
image

Is a hell of a lot better than this:
image

In other words, a broken image that isn't valid being displayed as an image which can't be rendered is better than an image not being recognized and ruining the post for others who just can't see or understand it.

@dessalines
Copy link
Member

@DraconicNEO open up an issue on the civitia repo, that they're serving the wrong content type for their images.

We merged a fix above, but there's some debate about whether the hack we merged above is appropriate.

@DraconicNEO
Copy link
Author

@DraconicNEO open up an issue on the civitia repo, that they're serving the wrong content type for their images.

We merged a fix above, but there's some debate about whether the hack we merged above is appropriate.

Great to hear, I already opened an issue in their repo civitai/civitai#1496 hopefully they'll get on that.

flamingo-cant-draw added a commit to flamingo-cant-draw/lemmy that referenced this issue Nov 25, 2024
flamingo-cant-draw added a commit to flamingo-cant-draw/lemmy that referenced this issue Nov 25, 2024
flamingo-cant-draw added a commit to flamingo-cant-draw/lemmy that referenced this issue Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants