Smidge 5: Nuglify-only, .NET 10, minimal APIs, and graceful 404 fixes - #223
Merged
Conversation
Move the whole solution from multi-targeting net8.0;net6.0 to a single net10.0 target for the Smidge 5 major release. - Directory.Build.props: TargetFrameworks -> net10.0, LangVersion -> latest - Smidge.Core: collapse the net6.0/net8.0 conditional package groups into a single Microsoft.Extensions.* 10.0.9 set - Smidge.InMemory: drop the duplicate Dazinator reference and the now-unneeded System.Text.Encodings.Web security pins (covered by the net10 shared framework) - Smidge.Tests: net10.0 - CI: setup-dotnet 6.0.x/8.0.x -> 10.0.x Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Smidge 5 drops its dependency on MVC for serving bundles. The two controllers and their action filters are replaced with minimal API endpoints and endpoint filters, so AddSmidge no longer forces MVC startup on the host. Tag helpers stay in the Smidge package (they keep the only Razor dependency). - Request models now use IHttpContextAccessor + Request.RouteValues instead of the obsolete IActionContextAccessor - The 4 action filters become IEndpointFilters (compression, expiry, not-modified, cache short-circuit), added outer-to-inner in the same order the MVC filter Order produced so behavior is preserved - SmidgeController -> SmidgeRequestHandler and NuglifySourceMapController -> NuglifySourceMapHandler: POCO handlers returning IResult - SmidgeStartup: drop AddMvcCore/AddApplicationPart and the IActionContextAccessor registration; register the handlers; UseSmidge maps three MapGet endpoints with the endpoint-filter chain - Remove the legacy useEndpointRouting/UseMvc branch and parameter (breaking) - Delete the unused BundleModelBinder Verified against the sample app: bundle/composite endpoints return 200 with the correct caching headers, If-None-Match yields 304, and tag helpers still render bundle URLs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Requests for composite files and Nuglify source maps could throw an unhandled FileNotFoundException that surfaced as a 500. Because the composite URL and source-map requests contain client-supplied values (and browsers request source maps lazily), this was easy to trigger repeatedly - a denial-of-service vector reported in #199 and the 500 seen for the notfound-map scenario in #185. Adds a non-throwing ICacheFileSystem.GetFileInfo(string) alongside the existing GetRequiredFileInfo (which stays throwing for genuine internal invariants). The composite and source-map request handlers now use the non-throwing lookup and return a graceful 404 (with a log entry) when a requested file is missing or stale. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
These types are implementation details invoked from the minimal API endpoints wired up in UseSmidge; they were never intended to be part of the public API surface. Marking them internal avoids committing to supporting them as public APIs in Smidge 5. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Covers the new non-throwing GetFileInfo alongside the throwing GetRequiredFileInfo for both the in-memory and physical cache file systems, locking in the behaviour the graceful 404 fix relies on. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Adds a Smidge.Integration.Tests project that self-hosts Smidge on Kestrel and exercises the same scenarios covered manually by the Smidge.Web sample views: production and debug bundles, dynamic composite files, source maps (served and gracefully 404'd), spoofed composite requests, empty bundles, conditional (304) requests and gzip compression. The whole suite runs twice via IClassFixture, once against the in-memory cache and once against the physical cache, to guard both code paths including the graceful 404 handling for missing cached/source-map files. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR implements the Smidge 5 major-release refactor: retargeting to .NET 10, removing MVC controller dependencies in favor of minimal API endpoints + endpoint filters, standardizing minification on Nuglify, and hardening attacker-reachable endpoints to return graceful 404s (instead of 500s) when cache/source-map files are missing.
Changes:
- Retargeted projects and CI to .NET 10 and updated build/test scaffolding accordingly.
- Replaced MVC controllers/action filters with minimal API endpoints, POCO request handlers, and endpoint filters.
- Introduced a non-throwing cache filesystem lookup path and added unit + integration tests to lock in 404-on-missing behavior.
Reviewed changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Smidge.Tests/Smidge.Tests.csproj | Retargets tests to net10.0 and references in-memory filesystem for new contract tests. |
| test/Smidge.Tests/CacheFileSystemTests.cs | Adds unit tests for throwing vs non-throwing cache filesystem lookup semantics. |
| test/Smidge.Integration.Tests/Smidge.Integration.Tests.csproj | New net10.0 integration test project for end-to-end Smidge pipeline validation. |
| test/Smidge.Integration.Tests/SmidgeTestApp.cs | Self-hosted Kestrel app wiring Smidge + helper endpoints for integration tests. |
| test/Smidge.Integration.Tests/SmidgeEndpointTests.cs | End-to-end tests for bundles, composites, source maps, 304s, gzip, and 404 hardening. |
| test/Smidge.Integration.Tests/SmidgeCacheEndpointTests.cs | Fixtures to run the integration suite against both in-memory and physical caches. |
| test/Smidge.Integration.Tests/wwwroot/Js/Bundle1/a1.js | Integration test JS input asset. |
| test/Smidge.Integration.Tests/wwwroot/Js/Bundle1/a2.js | Integration test JS input asset. |
| test/Smidge.Integration.Tests/wwwroot/Js/Bundle1/a3.min.js | Integration test “already minified” JS asset for minification conventions. |
| test/Smidge.Integration.Tests/wwwroot/Js/Folder/f1.js | Integration test JS folder asset for dynamic composite scenarios. |
| test/Smidge.Integration.Tests/wwwroot/Js/Folder/f2.js | Integration test JS folder asset for dynamic composite scenarios. |
| test/Smidge.Integration.Tests/wwwroot/Css/Bundle1/a1.css | Integration test CSS input asset. |
| test/Smidge.Integration.Tests/wwwroot/Css/Bundle1/a2.css | Integration test CSS input asset. |
| test/Smidge.Integration.Tests/wwwroot/Css/Folder/f1.css | Integration test CSS folder asset for dynamic composite scenarios. |
| test/Smidge.Integration.Tests/wwwroot/Css/notFoundMap.min.css | Integration test “already minified” CSS asset for source-map 404 behavior. |
| src/Directory.Build.props | Updates language version and central TFM settings to net10.0. |
| src/Smidge/SmidgeStartup.cs | Registers handler/filter types and maps Smidge endpoints via minimal APIs. |
| src/Smidge/Controllers/SmidgeRequestHandler.cs | Converts MVC controller logic into a POCO request handler returning IResult. |
| src/Smidge/Controllers/SmidgeEndpointFilters.cs | Replaces MVC action filters with endpoint filters for caching/etag/expiry/compression. |
| src/Smidge/Controllers/AddCompressionHeaderAttribute.cs | Removes obsolete MVC action filter attribute. |
| src/Smidge/Controllers/AddExpiryHeadersAttribute.cs | Removes obsolete MVC action filter attribute. |
| src/Smidge/Controllers/CheckNotModifiedAttribute.cs | Removes obsolete MVC action filter attribute. |
| src/Smidge/Controllers/CompositeFileCacheFilterAttribute.cs | Removes obsolete MVC action filter attribute. |
| src/Smidge/Nuglify/NuglifySourceMapHandler.cs | New POCO handler for Nuglify source-map requests with graceful 404 behavior. |
| src/Smidge/Nuglify/NuglifySourceMapController.cs | Removes MVC source-map controller in favor of POCO handler. |
| src/Smidge/Models/RequestModel.cs | Switches request parsing from IActionContextAccessor to IHttpContextAccessor/RouteValues. |
| src/Smidge/Models/BundleRequestModel.cs | Updates DI constructor signature for minimal API + IHttpContextAccessor. |
| src/Smidge/Models/CompositeFileModel.cs | Updates DI constructor signature for minimal API + IHttpContextAccessor. |
| src/Smidge/Models/BundleModelBinder.cs | Removes unused/unimplemented MVC model binder. |
| src/Smidge.Core/Cache/ICacheFileSystem.cs | Adds non-throwing GetFileInfo API alongside existing GetRequiredFileInfo. |
| src/Smidge.Core/Cache/PhysicalFileCacheFileSystem.cs | Implements the new GetFileInfo method for physical cache filesystem. |
| src/Smidge.Core/Smidge.Core.csproj | Updates Microsoft.Extensions.* package references for net10.0 targeting. |
| src/Smidge.InMemory/Smidge.InMemory.csproj | Cleans up legacy multi-target package conditionals after net10.0 retargeting. |
| src/Smidge.InMemory/MemoryCacheFileSystem.cs | Implements the new GetFileInfo method for in-memory cache filesystem. |
| src/Smidge.InMemory/ConfiguredCacheFileSystem.cs | Pass-through implementation of new GetFileInfo method for wrapped cache FS. |
| Smidge.sln | Adds integration test project and expands solution configurations/platforms. |
| .github/workflows/build.yml | Updates CI to install .NET 10 SDK. |
Comments suppressed due to low confidence (1)
src/Smidge/Controllers/SmidgeRequestHandler.cs:227
- The cache-path string for composite-part lookup is duplicated and the warning log currently outputs only the hash segment (without cache buster / extension), which makes diagnosing missing-cache cases harder. Consider computing the full requested cache path once and logging that exact value.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Moves the four endpoint filters out of SmidgeEndpointFilters.cs and the integration test fixtures out of SmidgeCacheEndpointTests.cs into individual files, and extracts the nested test helper types (TempFolder, pre-processor stubs) into their own files. No behaviour changes; purely a file layout refactor. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Migrates Smidge.sln to the XML-based Smidge.slnx solution format and updates the CI build workflow to reference it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- RequestModel: throw a clear InvalidOperationException when no active HttpContext is available instead of a NullReferenceException, and give the valueName ArgumentException a meaningful message. - CI: bump actions/setup-dotnet and actions/checkout to v4. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 50 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
src/Smidge/Controllers/SmidgeRequestHandler.cs:158
- The SemaphoreSlim is only released when the dictionary entry is removed. With concurrent requests, one request can remove the lock entry while other requests are still queued on the same SemaphoreSlim, causing later requests to never release and potentially deadlock any remaining waiters.
src/Smidge/Controllers/SmidgeRequestHandler.cs:151 - RequestModel.LastFileWriteTime is never updated after writing the newly generated bundle output to the cache. This means AddExpiryHeaders/CheckNotModified can emit an invalid Last-Modified (year 0001) and incorrectly evaluate If-Modified-Since on the first request after a cache miss.
src/Smidge/Controllers/SmidgeRequestHandler.cs:242 - RequestModel.LastFileWriteTime is never updated after writing the newly generated composite output to the cache. This can produce an invalid Last-Modified header (year 0001) and cause incorrect If-Modified-Since / 304 handling on the first request after a cache miss.
SmidgeRequire and NoopSmidgeRequire are framework-agnostic and only depend on types that already live in Smidge.Core (ISmidgeRequire, IBundleManager, IRequestHelper and the file models). Co-locating the implementations with their interface keeps the ASP.NET-free bundle configuration API entirely within the core layer. They remain internal; Core now grants InternalsVisibleTo to the Smidge project which consumes them from SmidgeHelper. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Per RFC 7232 a request that contains an If-None-Match header must ignore If-Modified-Since. The previous OR-based check could return 304 when a non-matching ETag was combined with an If-Modified-Since indicating the content was unchanged. The filter now evaluates ETag precedence first and only falls back to the modified-since date when no If-None-Match header is present. Adds an integration regression test covering the mismatched-ETag plus unmodified-since case. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Enables TreatWarningsAsErrors solution-wide via a repo-root Directory.Build.props (chained from src/Directory.Build.props so the src projects pick it up too) and resolves the outstanding build warnings: - Smidge.Web: replace the obsolete WebHost/IWebHost startup (ASPDEPR008) with the generic Host.CreateDefaultBuilder().ConfigureWebHostDefaults() pattern returning IHost. - Smidge.Tests: drop the redundant System.Diagnostics.TraceSource package reference flagged by NU1510. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
This is the Smidge 5 major release. It removes the built-in JSMin/CSSMin minifiers in favour of Nuglify, drops the MVC dependency, retargets to .NET 10, and fixes a long-standing class of
500errors that were a repeatable DoS vector when cached or source-map files are missing, stale, or spoofed (issues #185, #199, #200).What changed
Minification / packaging
Smidge.CoreintoSmidgeto simplify the package layout for the major version..NET 10 + minimal APIs (no more MVC)
UseSmidge, using POCO request handlers and endpoint filters instead of MVC action filters. This drops the MVC overhead entirely.internalso they are not part of the supported public API surface.Graceful 404 fix (DoS hardening)
ICacheFileSystem.GetFileInfo(string)alongside the existing throwingGetRequiredFileInfo./scfiles and the Nuglify source-map handler) now use the non-throwing lookup and return404instead of500when a requested file is missing, while genuine internal invariants (configured bundle source files) still throw as before. This is a cleaner alternative to the approach prototyped in Replaced FileNotFoundException with log instead (#199) #200 and also covers thenmapsource-map path.Tests
Smidge.Integration.Testsproject that mimics the scenarios previously only exercised manually via theSmidge.Websample views: production/debug bundles, dynamic composite files, source maps (served and gracefully 404'd), spoofed composite requests, empty bundles, conditional304requests, and gzip compression. The suite runs twice viaIClassFixture, once against the in-memory cache and once against the physical cache.Validation
Full solution suite is green: 65 unit tests + 22 integration tests = 87 passing, 0 failures.
Notes for reviewers
404if any requested part is missing rather than partially serving, which is what stops the500/DoS behaviour.Articulate.Nuglifyis intended to be marked obsolete separately; consumers should reference Nuglify directly.Smidge.Coreassembly no longer exists and several previously-public handler/filter types are now internal.Closes #199