Skip to content

HRINT-4835-2 - wrong images are displayed on certain posts#141

Merged
garayx merged 1 commit into
ayende:DOTNET8-MIGRATIONfrom
VladShpilMan:HRINT-4835-2
Jun 3, 2026
Merged

HRINT-4835-2 - wrong images are displayed on certain posts#141
garayx merged 1 commit into
ayende:DOTNET8-MIGRATIONfrom
VladShpilMan:HRINT-4835-2

Conversation

@VladShpilMan

Copy link
Copy Markdown

Issue:
https://issues.hibernatingrhinos.com/issue/HRINT-4835/ayende.com-Old-posts-Images-issues

Verified the collision fix. Took posts that reference image_thumb_1.png from different folders — with the old code all would resolve to the same flat doc images/image_thumb_1.png, showing the wrong image. With the fix each post gets its own unique image via full-path doc ID.

Opened each post and confirmed the image matches the content, then verified the image URL directly.

Post Image
/blog/4798/code-review-sharparchitecture-multitenant Code-Review-SharpArchitectur.MultiTenant_FF2F/image_thumb_1.png
/blog/4789/the-wages-of-sin-inverse-leaky-abstractions The-wages-of-sin-Inverse-leaky-abstracti_2259/image_thumb_1.png
/blog/4786/the-wages-of-sin-over-architecture-in-the-real-world The-wages-of-sin-Over-architecture-in-th_124F2/image_thumb_1.png
/blog/4787/the-wages-of-sin-re-creating-the-stored-procedure-api-in-c The-wages-of-sin-Re-creating-the_127AA/image_thumb_1.png
/blog/4784/architecting-in-the-pit-of-doom Architecting-in-the-pit-of-doom_12C75/image_thumb_1.png
/blog/4753/more-on-the-joy-of-support-my-trial-expired More-on-the-joy-of-support_E77D/image_thumb_1.png
/blog/4181/google-vs.-bing WindowsLiveWriter/Googlevs.Bing_1054F/image_thumb_1.png
/blog/4744/google-vs.-bing-again Windows-Live-Writer/Google-vs.-Bing_E768/image_thumb_1.png
/blog/3907/designing-a-document-database-aggregation WindowsLiveWriter/DesigningadocumentdatabaseAggregation_84C9/image_thumb_1.png
/blog/4184/google-vs.-bing-maps WindowsLiveWriter/Googlevs.BingMaps_BFC/image_thumb_1.png

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes incorrect images being served for certain legacy posts by making image document IDs unique per image path (rather than colliding on filename-only IDs), so posts that reference identically-named thumbnails in different folders resolve to the correct attachment.

Changes:

  • Update image lookup to load PostImage documents by full relative path (images/<relative/path>) instead of filename-only.
  • Update the DEBUG-only migration to generate PostImage doc IDs from the image’s relative path under wwwroot/images.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
RaccoonBlog.Web/Controllers/ImagesController.cs Switches image doc-id construction to include the full image path to avoid filename collisions.
RaccoonBlog.Web/Areas/Admin/Controllers/PostsController.cs Updates the debug migration to store images under path-based doc IDs (relative to wwwroot/images).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +23 to +26
if (string.IsNullOrEmpty(imagePath))
return NotFound();
var docId = "images/" + fileName;

var docId = "images/" + imagePath.ToLowerInvariant();

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is PR2, so we need to remember PR1. In PR1, we added ImageUrlRewriter. The imagePath comes from . ImageUrlRewriter always uses clean forward slashes /. So bad path with \ cannot happen here.

@garayx or do you think we should do what Copilot suggests?

Comment on lines 393 to 395
[HttpGet("admin/posts/migrate-images")]
[AllowAnonymous]
[AllowAnonymous]
public IActionResult MigrateOldImages([FromServices] IWebHostEnvironment env)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This endpoint is #if DEBUG only and never compiled into Release builds.

Comment on lines 416 to 420
var batchFiles = allFiles.Skip(i).Take(batchSize).ToList();
var openStreams = new List<FileStream>();
var openStreams = new List<FileStream>();

using (var fileSession = DocumentStore.OpenSession())
{

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@garayx garayx merged commit 67cdc3b into ayende:DOTNET8-MIGRATION Jun 3, 2026
1 check passed
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

Successfully merging this pull request may close these issues.

3 participants