Fix directory bundles pulling in wrong-type files (e.g. .js.gz) - #225
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>
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>
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>
When a bundle points at a bare folder (e.g. bundles.CreateJs("x", "~/Js/Bundle2")),
Smidge normalized the glob to "*.*" which matched every file in the folder regardless
of extension. Generated artifacts such as .js.gz or .map files, or files of the wrong
type, were then handed to the wrong pre-processor - e.g. a gzipped file passed to
Nuglify producing "JS1014: Invalid character".
Directory expansion is now constrained to the bundle's WebFileType extension (.js/.css)
via a new GetMatchingFiles(filePattern, WebFileType) overload. The untyped overload
keeps its previous "*.*" behaviour for back-compat.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Owner
Author
|
lgtm |
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
1 participant
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.
Problem
When a bundle points at a bare folder, e.g.:
Smidge normalized the folder into the glob
~/Js/Bundle2/*.*, which matched every file in the directory regardless of extension. Generated artifacts such as.js.gzor.mapfiles — or files of the wrong type entirely — were then included in the bundle and handed to the wrong pre-processor.Concretely, a gzipped
b1.js.gzpassed to Nuglify produced:(the "invalid characters" being the gzip binary header).
Fix
Directory expansion is now constrained to the bundle's
WebFileType:GetMatchingFiles(string filePattern, WebFileType fileType)toISmidgeFileSystem. For a bare directory it globs*.jsor*.cssinstead of*.*.BundleFileSetGeneratorandFileBatchernow pass the bundle'sDependencyType.GetMatchingFiles(string)retains its previous*.*behaviour for backwards compatibility.~/Js/lib.tsfor custom pre-processors) are unaffected — only the bare-directory shorthand is constrained.Tests
SmidgeFileSystemTests.GetMatchingFiles_Directory_Constrains_To_FileType, which verifies that.js.gz,.map, and wrong-type files are excluded from a directory bundle, and that the untyped overload still matches everything.