Migrate ASP.NET MVC 5 / .NET 4.5.1 app to ASP.NET Core on .NET 9#22
Migrate ASP.NET MVC 5 / .NET 4.5.1 app to ASP.NET Core on .NET 9#22devin-ai-integration[bot] wants to merge 7 commits into
Conversation
…ET 9 - Convert all projects to SDK-style net9.0; remove packages.config/Web.config/App.config - Replace EF6 with EF Core 9 (DbContext, Fluent API, EnsureCreated + seed) - Add GenericServices compat library (EF Core + AutoMapper) preserving the original EfGenericDto/ISuccessOrErrors service surface - Replace Global.asax/OWIN/Autofac/SignalR with Program.cs minimal hosting + built-in DI - Migrate controllers to ASP.NET Core MVC ([FromServices] action injection) - Migrate Razor views (_ViewImports, tag helpers, direct link/script tags, wwwroot) - Convert Tests to net9.0 xUnit using EF Core in-memory (12 passing) - Use SQLite for local dev (SqlServer provider selectable via config) Co-Authored-By: Dillon Vargo <dillonvargo@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
ASP.NET Core's validation visitor invokes all model getters during model binding. On the posted-back Post DTO the Tags collection is null, so the TagNames computed getter (Tags.Select(...)) threw ArgumentNullException on Post Create/Edit. Guard the getter to return empty when Tags is null. Co-Authored-By: Dillon Vargo <dillonvargo@gmail.com>
Co-Authored-By: Dillon Vargo <dillonvargo@gmail.com>
Runtime test results — migrated app on .NET 9Ran the migrated app locally ( Post Create with secondary data (the previously-broken area) — PASSThe Create form populates the Bloggers dropdown (4) + Tags multiselect (8); submitting creates the post with the correct blogger and tag names.
Tag CRUD (create / validate / edit / delete) — PASS
Post details persisted + regression (Blogs list, PostsAsync) — PASS
Assertions
Bug found & fixed during testingPost Create/Edit initially crashed with No CI is configured on this repo, so there were no checks to wait on. Devin session: https://app.devin.ai/sessions/97ae13da2ed64c0c821a10a57d4cf380 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| public string ErrorsAsHtml() | ||
| { | ||
| var sb = new StringBuilder(); | ||
| foreach (var error in _errors) | ||
| { | ||
| if (sb.Length > 0) | ||
| sb.Append("<br/>"); | ||
| sb.Append(error.ErrorMessage); | ||
| } | ||
| return sb.ToString(); | ||
| } |
There was a problem hiding this comment.
🔴 XSS vulnerability: ErrorsAsHtml() does not HTML-encode user-controlled content before embedding in HTML
The newly written ErrorsAsHtml() method at GenericServices/Core/SuccessOrErrors.cs:95-105 builds an HTML string with <br/> separators but appends raw error.ErrorMessage without HTML-encoding. Error messages can contain user-controlled data — for example, SampleWebAppDb.ExtraValidation (DataLayer/DataClasses/SampleWebAppDb.cs:64) interpolates tagToCheck.Name directly into the message. Controllers then store this in TempData (e.g., SampleWebApp/Controllers/TagsController.cs:111), and multiple views render it with @Html.Raw(TempData["errorMessage"]) (e.g., SampleWebApp/Views/Tags/Index.cshtml:15). A user who enters a tag name like <script>alert('xss')</script> would have that script executed in the browser of any user viewing the error page.
Prompt for agents
The ErrorsAsHtml() method in GenericServices/Core/SuccessOrErrors.cs (lines 95-105, and the duplicate at lines 165-175 in the generic SuccessOrErrors<T> class) appends error.ErrorMessage directly into an HTML string without encoding. Error messages can contain user-supplied data (e.g. tag names from SampleWebAppDb.ExtraValidation). The result is rendered via @Html.Raw() in multiple Razor views (Blogs/Index, Posts/Index, PostsAsync/Index, Tags/Index, TagsAsync/Index).
To fix: HTML-encode each error message before appending it. Use System.Net.WebUtility.HtmlEncode(error.ErrorMessage) instead of bare error.ErrorMessage in both ErrorsAsHtml() implementations. This ensures the <br/> separators are still treated as markup while the message text is safely escaped.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed and fixed in 0f39a9b. This is a real stored XSS: SampleWebAppDb.ExtraValidation interpolates the user-supplied tag name into the error message → ErrorsAsHtml() concatenated it raw → controllers store it in TempData["errorMessage"] → the index views render it with @Html.Raw(...). So a tag named <script>...</script> would execute.
Fix: both ErrorsAsHtml() overloads (non-generic + generic) now WebUtility.HtmlEncode each error.ErrorMessage before appending, while keeping the <br/> separators as literal markup. Added a regression test (ErrorsAsHtmlEncodesMessageButKeepsLineBreaks) asserting <script> is escaped to <script> but <br/> is preserved. All 14 tests pass.
Error messages can contain user input (e.g. tag names) and are rendered via @Html.Raw in the index views. Encode each message with WebUtility.HtmlEncode while keeping the <br/> separators as markup. Co-Authored-By: Dillon Vargo <dillonvargo@gmail.com>
…olled Recursion (SNYK-DOTNET-AUTOMAPPER-15602868) Snyk flagged AutoMapper 13.0.1 for an Uncontrolled Recursion vuln; the fixed range starts at 15.1.3. Updated both project references and adapted to the AutoMapper 15 API: use the built-in AddAutoMapper DI extension in ServiceLayerConfig and pass NullLoggerFactory to MapperConfiguration in the test helper (the ctor now requires an ILoggerFactory). Co-Authored-By: Dillon Vargo <dillonvargo@gmail.com>
Snyk's Git-based PR checks (security/snyk, license/snyk) error out on the SDK-style PackageReference projects because, after dropping packages.config in the migration, there was no committed dependency graph for Snyk to parse (it does not run a restore). Enable RestorePackagesWithLockFile via Directory.Build.props and commit the generated packages.lock.json for each project so the resolved dependency tree is available to scanners. Co-Authored-By: Dillon Vargo <dillonvargo@gmail.com>
The lock files did not resolve the Snyk PR-check error: Snyk's Git-based importer does not recognize packages.lock.json as a NuGet manifest and never runs a restore, so it still has no resolved dependency graph. Reverting to avoid the RestorePackagesWithLockFile maintenance burden; the real fix is on the Snyk integration side (scan via the CLI after dotnet restore). Co-Authored-By: Dillon Vargo <dillonvargo@gmail.com>
Summary
Full-framework migration of the SampleMvcWebApp blogging demo from ASP.NET MVC 5 / .NET Framework 4.5.1 to ASP.NET Core on net9.0. All 5 projects are now SDK-style, EF6 is replaced by EF Core 9, and the legacy
GenericServices 1.0.9package is replaced by an in-repo compat library so controllers/views/DTOs keep their original shape.Why a compat library instead of
EfCore.GenericServicesThe app is tightly coupled to the old GenericServices surface:
EfGenericDtooverride hooks (SetupSecondaryData/CreateDataFromDto/UpdateDataFromDto), action-parameter service injection, and theISuccessOrErrorsresult type used throughout controllers.EfCore.GenericServicesexposes a fundamentally differentICrudServicesAPI and cannot reproduce these patterns. Per the task note ("you may need to implement some service methods manually"), a faithful EF Core + AutoMapper reimplementation of the original interfaces was the lowest-risk path. New project:GenericServices/.Key changes by layer
.csproj→ SDK-stylenet9.0; removed everypackages.config,Web.config,App.config,Global.asax,App_Start/, Autofac modules,AssemblyInfo.cs. Addedappsettings.json+launchSettings.json.SampleWebAppDb : DbContext, IGenericServicesDbContextwith constructorDbContextOptions, Fluent API inOnModelCreating(Post→Blog FK, Post↔Tag many-to-many),SaveChangeshook forLastUpdated, andExtraValidationenforcingTag.Sluguniqueness (was EF6ValidateEntity).DataLayerInitialisenow usesEnsureCreated()+ XML seed.EfGenericDto<TData,TDto>(+ async),ISuccessOrErrors/<T>,SaveChangesWithChecking(DataAnnotations +IValidatableObject+ExtraValidation), 13 service interfaces/impls projecting via AutoMapperProjectTo.Program.csminimal hosting; built-in DI viaAddServiceLayer(); DB provider selectable through config (Sqlitedefault for local dev,SqlServeravailable). Controllers inherit Core MVC and use[FromServices]for action-parameter service injection;MvcHtmlString→raw string +@Html.Raw._ViewImports.cshtml(tag helpers + usings),_Layoutuses direct<link>/<script>instead of bundles, removedViews/Web.config, static assets moved towwwroot/.net9.0xUnit on EF Core InMemory; old Autofac/SignalR/System.Web tests (which tested removed infrastructure) replaced with focused data-layer + service-layer CRUD tests. 12 passing.Verification
dotnet buildclean across the solution;dotnet test→ 12/12 pass.dotnet run), seeds SQLite, and all pages return 200 (Home, Posts/Tags/Blogs + Async variants, Create/Edit/Details, secondary-data dropdowns populate).Notes
DelegateDecompiler/Mono.Reflectiondropped — the compat library computes aggregate fields (e.g.PostsCount) via AutoMapper projection conventions instead.Link to Devin session: https://app.devin.ai/sessions/97ae13da2ed64c0c821a10a57d4cf380
Requested by: @dillonvargo
Devin Review