diff --git a/src/ImageMapper.Api/Controllers/ImagesController.cs b/src/ImageMapper.Api/Controllers/ImagesController.cs index ce9eb91..71bce55 100644 --- a/src/ImageMapper.Api/Controllers/ImagesController.cs +++ b/src/ImageMapper.Api/Controllers/ImagesController.cs @@ -1,9 +1,7 @@ -using ImageMapper.Api.Exceptions; using ImageMapper.Api.Services; using ImageMapper.Models; using Microsoft.AspNetCore.Mvc; using Serilog; -using System.Web; namespace ImageMapper.Api.Controllers; @@ -26,26 +24,15 @@ public async Task> GetCount(CancellationToken ct) return Ok(count); } - [HttpGet("raw/{**relativePath}")] - public async Task GetRaw(string relativePath, CancellationToken ct) + [HttpGet("raw/{id}")] + public async Task GetRaw(string id, CancellationToken ct) { - // URL-decode the path to handle encoded path separators (%2F -> /) - var decodedPath = HttpUtility.UrlDecode(relativePath); + Log.Debug("GET /api/images/raw/{Id} - Retrieving image", id); - Log.Debug("GET /api/images/raw/{RelativePath} - Retrieving image", decodedPath); + var bytes = await _svc.GetImageBytesAsync(id, ct); + if (bytes == null) + return NotFound(); - try - { - var bytes = await _svc.GetImageBytesAsync(decodedPath, ct); - if (bytes == null) - return NotFound(); - - return File(bytes, "application/octet-stream"); - } - catch (PathTraversalException ex) - { - Log.Warning("Path traversal rejected: {Message}", ex.Message); - return BadRequest("Invalid path"); - } + return File(bytes, "application/octet-stream"); } } diff --git a/src/ImageMapper.Api/Exceptions/PathTraversalException.cs b/src/ImageMapper.Api/Exceptions/PathTraversalException.cs deleted file mode 100644 index 8bdc694..0000000 --- a/src/ImageMapper.Api/Exceptions/PathTraversalException.cs +++ /dev/null @@ -1,17 +0,0 @@ -namespace ImageMapper.Api.Exceptions; - -/// -/// Thrown when a path traversal attempt is detected. -/// -public class PathTraversalException : ArgumentException -{ - public PathTraversalException() : base("Path traversal detected") { } - - public PathTraversalException(string message) : base(message) { } - - public PathTraversalException(string message, Exception innerException) - : base(message, innerException) { } - - public PathTraversalException(string message, string paramName) - : base(message, paramName) { } -} diff --git a/src/ImageMapper.Api/Services/IImageService.cs b/src/ImageMapper.Api/Services/IImageService.cs index c3a55a1..cde6499 100644 --- a/src/ImageMapper.Api/Services/IImageService.cs +++ b/src/ImageMapper.Api/Services/IImageService.cs @@ -1,4 +1,3 @@ -using ImageMapper.Api.Exceptions; using ImageMapper.Models; namespace ImageMapper.Api.Services; @@ -15,18 +14,15 @@ public interface IImageService IAsyncEnumerable GetImagesAsync(CancellationToken ct = default); /// - /// Asynchronously retrieves the image data as a byte array from the specified relative path. + /// Asynchronously retrieves the image data as a byte array from the specified image ID. /// - /// This method is intended for use in scenarios where image data needs to be loaded - /// asynchronously, such as in UI applications. Ensure that the relative path is correctly specified to avoid - /// errors. - /// The relative path to the image file. This path must be valid and accessible; otherwise, an exception may be - /// thrown. + /// The image ID is a unique identifier generated from the image's full path. + /// This prevents the frontend from accessing file system paths. + /// The unique image ID /// A cancellation token that can be used to cancel the operation. The default value is CancellationToken.None. /// A task that represents the asynchronous operation. The task result contains a byte array of the image data, or /// null if the image could not be found. - /// Thrown when the provided relative path is invalid or attempts to traverse outside the allowed directory." - Task GetImageBytesAsync(string relativePath, CancellationToken ct = default); + Task GetImageBytesAsync(string id, CancellationToken ct = default); /// /// Asynchronously retrieves the total count of image files. diff --git a/src/ImageMapper.Api/Services/ImageService.cs b/src/ImageMapper.Api/Services/ImageService.cs index 9f1b276..a07281d 100644 --- a/src/ImageMapper.Api/Services/ImageService.cs +++ b/src/ImageMapper.Api/Services/ImageService.cs @@ -1,8 +1,8 @@ -using ImageMapper.Api.Exceptions; using ImageMapper.Models; using MetadataExtractor; using MetadataExtractor.Formats.Exif; using Serilog; +using System.Buffers.Text; using System.Runtime.CompilerServices; namespace ImageMapper.Api.Services; @@ -12,6 +12,10 @@ public class ImageService : IImageService private readonly IConfiguration _config; private readonly string _imagesRoot; + // In-memory lookup from ID to full file path + private static readonly Dictionary IdToPathMapping = new(StringComparer.OrdinalIgnoreCase); + private static readonly SemaphoreSlim MappingSem = new(1, 1); + private static readonly string[] ValidExtensions = [".jpg", ".jpeg", ".png", ".tif", ".tiff", ".nef"]; public ImageService(IConfiguration config) @@ -33,8 +37,8 @@ public async IAsyncEnumerable GetImagesAsync([EnumeratorCancellation] foreach (string f in files) { ct.ThrowIfCancellationRequested(); - var rel = Path.GetRelativePath(_imagesRoot, f).Replace("\\", "/"); - var info = new ImageInfo { RelativePath = rel, FileName = Path.GetFileName(f) }; + var id = await GenerateIdForPath(f, ct); + var info = new ImageInfo { Id = id, FileName = Path.GetFileName(f) }; try { var directories = ImageMetadataReader.ReadMetadata(f); @@ -60,28 +64,45 @@ public async IAsyncEnumerable GetImagesAsync([EnumeratorCancellation] } } - public async Task GetImageBytesAsync(string relativePath, CancellationToken ct = default) + private static async Task GenerateIdForPath(string fullPath, CancellationToken ct = default) { - // Normalize path separators to OS-specific - var normalized = relativePath.Replace('/', Path.DirectorySeparatorChar).Replace('\\', Path.DirectorySeparatorChar); - var full = Path.Combine(_imagesRoot, normalized); - var fullPath = Path.GetFullPath(full); - var rootPath = Path.GetFullPath(_imagesRoot); - - // Ensure rootPath ends with separator for proper boundary checking - if (!rootPath.EndsWith(Path.DirectorySeparatorChar)) - rootPath += Path.DirectorySeparatorChar; - - // Validate that the resolved path is within the root directory - if (!fullPath.StartsWith(rootPath, StringComparison.OrdinalIgnoreCase)) + // Generate a unique ID based on the full path + var id = Base64Url.EncodeToString(System.Security.Cryptography.SHA256.HashData(System.Text.Encoding.UTF8.GetBytes(fullPath))); + + await MappingSem.WaitAsync(ct); + try + { + IdToPathMapping[id] = fullPath; + } + finally { - throw new PathTraversalException("Path traversal detected", nameof(relativePath)); + MappingSem.Release(); } - if (!File.Exists(fullPath)) - return null; + return id; + } + + public async Task GetImageBytesAsync(string id, CancellationToken ct = default) + { + await MappingSem.WaitAsync(ct); + try + { + if (!IdToPathMapping.TryGetValue(id, out var fullPath)) + return null; + + if (!File.Exists(fullPath)) + { + // Remove stale mapping + IdToPathMapping.Remove(id); + return null; + } - return await File.ReadAllBytesAsync(fullPath, ct); + return await File.ReadAllBytesAsync(fullPath, ct); + } + finally + { + MappingSem.Release(); + } } public Task GetImageCountAsync(CancellationToken ct = default) diff --git a/src/ImageMapper.Models/ImageInfo.cs b/src/ImageMapper.Models/ImageInfo.cs index ed0c988..bb9b8e7 100644 --- a/src/ImageMapper.Models/ImageInfo.cs +++ b/src/ImageMapper.Models/ImageInfo.cs @@ -2,7 +2,7 @@ namespace ImageMapper.Models; public class ImageInfo { - public string RelativePath { get; set; } = string.Empty; + public string Id { get; set; } = string.Empty; public string FileName { get; set; } = string.Empty; public double? Latitude { get; set; } public double? Longitude { get; set; } diff --git a/src/ImageMapper.Tests/ImagesApiTest.cs b/src/ImageMapper.Tests/ImagesApiTest.cs index bbebfe7..9fa08b6 100644 --- a/src/ImageMapper.Tests/ImagesApiTest.cs +++ b/src/ImageMapper.Tests/ImagesApiTest.cs @@ -1,4 +1,3 @@ -using ImageMapper.Api.Exceptions; using ImageMapper.Api.Services; using ImageMapper.Models; using Microsoft.Extensions.Configuration; @@ -41,29 +40,6 @@ public void OneTimeTearDown() Directory.Delete(_testImagesDirectory, recursive: true); } - [Test] - [TestCase("../../etc/passwd")] - [TestCase("../../../windows/system32/config/sam")] - [TestCase("..\\..\\..\\windows\\system32\\config\\sam")] - [TestCase("images/../../etc/passwd")] - [TestCase("subfolder/../../../../sensitive.txt")] - [TestCase("valid-image.jpg/../../../etc/passwd")] - public void GetImageBytesAsyncRejectsPathTraversalAttempts(string traversalPath) - { - // Arrange - var config = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { { "ImageFolder", _testImagesDirectory } }) - .Build(); - var service = new ImageService(config); - - // Act & Assert - var ex = Assert.ThrowsAsync( - async () => await service.GetImageBytesAsync(traversalPath)); - - Assert.That(ex.ParamName, Is.EqualTo("relativePath")); - Assert.That(ex.Message, Contains.Substring("Path traversal detected")); - } - [Test] public async Task GetImageBytesAsyncReturnsValidImageBytes() { @@ -73,8 +49,18 @@ public async Task GetImageBytesAsyncReturnsValidImageBytes() .Build(); var service = new ImageService(config); + // Get the image ID first + var images = new List(); + await foreach (var image in service.GetImagesAsync()) + { + if (image.FileName == "test-image.jpg") + images.Add(image); + } + Assert.That(images, Has.Count.GreaterThan(0), "test-image.jpg not found"); + var testImageId = images[0].Id; + // Act - var bytes = await service.GetImageBytesAsync("test-image.jpg"); + var bytes = await service.GetImageBytesAsync(testImageId); // Assert Assert.That(bytes, Is.Not.Null); @@ -91,17 +77,17 @@ public async Task GetImageBytesAsyncReturnsNullForNonExistentFile() .Build(); var service = new ImageService(config); - // Act - var bytes = await service.GetImageBytesAsync("nonexistent.jpg"); + // Act - use a fake ID that doesn't exist + var bytes = await service.GetImageBytesAsync("nonexistent-id-12345"); // Assert Assert.That(bytes, Is.Null); } [Test] - [TestCase("subfolder/nested-image.jpg")] - [TestCase("subfolder/deep/deep-image.png")] - public async Task GetImageBytesAsyncReturnsValidImageBytesFromSubfolders(string relativePath) + [TestCase("nested-image.jpg")] + [TestCase("deep-image.png")] + public async Task GetImageBytesAsyncReturnsValidImageBytesFromSubfolders(string fileName) { // Arrange var config = new ConfigurationBuilder() @@ -109,33 +95,24 @@ public async Task GetImageBytesAsyncReturnsValidImageBytesFromSubfolders(string .Build(); var service = new ImageService(config); + // Get the image ID first + var images = new List(); + await foreach (var image in service.GetImagesAsync()) + { + if (image.FileName == fileName) + images.Add(image); + } + Assert.That(images, Has.Count.GreaterThan(0), $"{fileName} not found"); + var imageId = images[0].Id; + // Act - var bytes = await service.GetImageBytesAsync(relativePath); + var bytes = await service.GetImageBytesAsync(imageId); // Assert Assert.That(bytes, Is.Not.Null); Assert.That(bytes, Has.Length.GreaterThan(0)); } - [Test] - [TestCase("subfolder/nonexistent.jpg")] - [TestCase("subfolder/deep/missing-image.png")] - [TestCase("nonexistent-folder/image.jpg")] - public async Task GetImageBytesAsyncReturnsNullForNonExistentFilesInSubfolders(string relativePath) - { - // Arrange - var config = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { { "ImageFolder", _testImagesDirectory } }) - .Build(); - var service = new ImageService(config); - - // Act - var bytes = await service.GetImageBytesAsync(relativePath); - - // Assert - Assert.That(bytes, Is.Null); - } - [Test] public async Task GetImagesAsyncReturnsAllImagesFromDirectory() { @@ -159,28 +136,6 @@ public async Task GetImagesAsyncReturnsAllImagesFromDirectory() Assert.That(images.Select(i => i.FileName), Does.Contain("deep-image.png")); } - [Test] - public async Task GetImagesAsyncReturnsCorrectRelativePaths() - { - // Arrange - var config = new ConfigurationBuilder() - .AddInMemoryCollection(new Dictionary { { "ImageFolder", _testImagesDirectory } }) - .Build(); - var service = new ImageService(config); - - // Act - var images = new List(); - await foreach (var image in service.GetImagesAsync()) - { - images.Add(image); - } - - // Assert - Assert.That(images.Select(i => i.RelativePath), Does.Contain("test-image.jpg")); - Assert.That(images.Select(i => i.RelativePath), Does.Contain("subfolder/nested-image.jpg")); - Assert.That(images.Select(i => i.RelativePath), Does.Contain("subfolder/deep/deep-image.png")); - } - [Test] public async Task GetImagesAsyncReturnsEmptyListForNonExistentDirectory() { diff --git a/src/ImageMapper.Tests/IntegrationTest.cs b/src/ImageMapper.Tests/IntegrationTest.cs index b20b396..4cabf0c 100644 --- a/src/ImageMapper.Tests/IntegrationTest.cs +++ b/src/ImageMapper.Tests/IntegrationTest.cs @@ -112,32 +112,6 @@ public async Task GetImagesListReturnsAllImages() Assert.That(images.Select(i => i.FileName), Does.Contain("nested-image.png")); } - [Test] - public async Task GetImagesListReturnsCorrectRelativePaths() - { - // Arrange - using var cts = new CancellationTokenSource(DefaultTimeout); - var cancellationToken = cts.Token; - - await using var app = await BuildAndStartAppAsync(cancellationToken, _testImagesDirectory); - using var httpClient = app.CreateHttpClient("imagemapper-api"); - await app.ResourceNotifications.WaitForResourceHealthyAsync("imagemapper-web", cancellationToken).WaitAsync(DefaultTimeout, cancellationToken); - - var fetcher = new ImageItemFetcher(httpClient); - - // Act - var images = new List(); - await foreach (var image in fetcher.Fetch(cancellationToken)) - { - if (image != null) - images.Add(image); - } - - // Assert - Assert.That(images.Select(i => i.RelativePath), Does.Contain("test-image.jpg")); - Assert.That(images.Select(i => i.RelativePath), Does.Contain("subfolder/nested-image.png")); - } - [Test] public async Task GetImagesListReturnsEmptyWhenNoImagesExist() { @@ -180,18 +154,31 @@ public async Task GetRawImageReturnsExistingFile() // Arrange using var cts = new CancellationTokenSource(DefaultTimeout); var cancellationToken = cts.Token; - + await using var app = await BuildAndStartAppAsync(cancellationToken, _testImagesDirectory); // Act - using var httpClient = app.CreateHttpClient("imagemapper-web"); + using var httpClientApi = app.CreateHttpClient("imagemapper-api"); await app.ResourceNotifications.WaitForResourceHealthyAsync("imagemapper-web", cancellationToken).WaitAsync(DefaultTimeout, cancellationToken); - using var response = await httpClient.GetAsync("/api/images/raw/test-image.jpg", cancellationToken); + + var fetcher = new ImageItemFetcher(httpClientApi); + var images = new List(); + await foreach (var image in fetcher.Fetch(cancellationToken)) + { + if (image != null) + images.Add(image); + } + + var testImageId = images.FirstOrDefault(i => i.FileName == "test-image.jpg")?.Id; + Assert.That(testImageId, Is.Not.Null, "test-image.jpg not found"); + + using var httpClientWeb = app.CreateHttpClient("imagemapper-web"); + using var response = await httpClientWeb.GetAsync($"/api/images/raw/{testImageId}", cancellationToken); // Assert Assert.That(response.StatusCode, Is.EqualTo(HttpStatusCode.OK)); Assert.That(response.Content.Headers.ContentType?.MediaType, Is.EqualTo("application/octet-stream")); - + var content = await response.Content.ReadAsByteArrayAsync(cancellationToken); Assert.That(content, Has.Length.GreaterThan(0)); Assert.That(content[0], Is.EqualTo(0xFF)); // JPEG magic byte @@ -203,25 +190,25 @@ public async Task GetRawImageReturnsNotFoundForNonExistentFile() // Arrange using var cts = new CancellationTokenSource(DefaultTimeout); var cancellationToken = cts.Token; - + await using var app = await BuildAndStartAppAsync(cancellationToken, _testImagesDirectory); // Act using var httpClient = app.CreateHttpClient("imagemapper-web"); await app.ResourceNotifications.WaitForResourceHealthyAsync("imagemapper-web", cancellationToken).WaitAsync(DefaultTimeout, cancellationToken); - using var response = await httpClient.GetAsync("/api/images/raw/nonexistent-image.jpg", cancellationToken); + using var response = await httpClient.GetAsync("/api/images/raw/nonexistent-id", cancellationToken); // Assert Assert.That(response.StatusCode, Is.EqualTo(HttpStatusCode.NotFound)); } [Test] - public async Task GetRawImageReturnsBadRequestForEmptyPath() + public async Task GetRawImageReturnsNotFoundForEmptyPath() { // Arrange using var cts = new CancellationTokenSource(DefaultTimeout); var cancellationToken = cts.Token; - + await using var app = await BuildAndStartAppAsync(cancellationToken); // Act @@ -230,7 +217,7 @@ public async Task GetRawImageReturnsBadRequestForEmptyPath() using var response = await httpClient.GetAsync("/api/images/raw/", cancellationToken); // Assert - Assert.That(response.StatusCode, Is.EqualTo(HttpStatusCode.BadRequest)); + Assert.That(response.StatusCode, Is.EqualTo(HttpStatusCode.NotFound)); } } } diff --git a/src/ImageMapper.Web/Client/ImageItemFetcher.cs b/src/ImageMapper.Web/Client/ImageItemFetcher.cs index 0585406..13b6cfb 100644 --- a/src/ImageMapper.Web/Client/ImageItemFetcher.cs +++ b/src/ImageMapper.Web/Client/ImageItemFetcher.cs @@ -27,12 +27,12 @@ public async Task FetchImageCount(CancellationToken ct = default) /// Fetch image content streamed to the caller without buffering the entire response in memory. /// Caller is responsible for disposing the returned Stream when finished. /// - /// + /// The unique image ID /// /// - public async Task FetchRawImageStream(string relativePath, CancellationToken ct) + public async Task FetchRawImageStream(string id, CancellationToken ct) { - var requestUrl = $"/api/images/raw/{Uri.EscapeDataString(relativePath)}"; + var requestUrl = $"/api/images/raw/{id}"; var response = await httpClient.GetAsync(requestUrl, HttpCompletionOption.ResponseHeadersRead, ct); if (!response.IsSuccessStatusCode) return null; diff --git a/src/ImageMapper.Web/Components/Pages/Home.razor.cs b/src/ImageMapper.Web/Components/Pages/Home.razor.cs index f3747b1..1986ea9 100644 --- a/src/ImageMapper.Web/Components/Pages/Home.razor.cs +++ b/src/ImageMapper.Web/Components/Pages/Home.razor.cs @@ -43,7 +43,7 @@ await JS.InvokeVoidAsync("addMarkerToMap", image.FileName, image.Latitude, image.Longitude, - Url = $"/api/images/raw/{Uri.EscapeDataString(image.RelativePath)}" + Url = $"/api/images/raw/{image.Id}" }); imagesLoaded++; diff --git a/src/ImageMapper.Web/Controllers/ImagesController.cs b/src/ImageMapper.Web/Controllers/ImagesController.cs index 9cd2048..f5c08b0 100644 --- a/src/ImageMapper.Web/Controllers/ImagesController.cs +++ b/src/ImageMapper.Web/Controllers/ImagesController.cs @@ -1,6 +1,5 @@ using ImageMapper.Web.Client; using Microsoft.AspNetCore.Mvc; -using System.Web; namespace ImageMapper.Web.Controllers { @@ -8,15 +7,10 @@ namespace ImageMapper.Web.Controllers [ApiController] public class ImagesController(ImageItemFetcher imageFetcher) : ControllerBase { - [HttpGet("raw/{**relativePath}")] - public async Task GetRaw(string relativePath, CancellationToken ct) + [HttpGet("raw/{id}")] + public async Task GetRaw(string id, CancellationToken ct) { - if (string.IsNullOrWhiteSpace(relativePath)) - return BadRequest("Relative path cannot be empty."); - - // URL-decode the path to handle encoded path separators (%2F -> /) - var decodedPath = HttpUtility.UrlDecode(relativePath); - var stream = await imageFetcher.FetchRawImageStream(decodedPath, ct); + var stream = await imageFetcher.FetchRawImageStream(id, ct); if (stream == null) return NotFound();