Commit 30c0788
authored
Fix the flaws found in a full-project audit (#7)
* Overhaul generation, samples, and SSR configuration
* Generate sample types before JavaScript checks
* Async SSR contract, SSR authorization, and package licensing
ISvelteSsrEngine.Render becomes RenderAsync, and ISvelteSsrFetchHandler
gains HandleAsync. A server render can await remote queries, which hit
databases and HTTP services; the old synchronous contract forced
sync-over-async at every layer and blocked a request thread for the whole
await chain.
- CliSsrEngine (Node/Bun) is now genuinely async, and probes its CLI on
first render instead of in the constructor.
- Jint cannot suspend a running script, so its fetch bridge must block.
RenderAsync moves that onto a worker gated by MaxConcurrentRenders, so
the request thread is released and blocked threads stay bounded.
- Pooled Jint engines outlive a request, so the fetch shim reads a
thread-static token for the render in flight rather than closing over
the token the engine was created with.
- SveltePage prerenders in the page filter, keeping @Model.Svelte() in
.cshtml working; SvelteAsync()/SvelteHeadAsync() render on demand.
MVC views use @await Html.SvelteAsync(Model).
The Jint SSR bridge dispatches descriptors directly and never passes
through routing, so RequireAuthorization() on the endpoints did not apply
to it: an anonymous page view would run a protected query and bake the
result into the HTML. Authorization is now descriptor-level data that both
transports read - MapSvelteRemote projects it onto per-method endpoints,
and the bridge evaluates the same metadata via IAuthorizationService.
Also: MIT LICENSE and package license metadata (the packages shipped with
none), Source Link and symbol packages, all three packages packed in CI,
ILogger instead of Console for SSR output, ordinal manifest lookup, and
JsonDocuments are disposed.
Tests: 132 -> 147, covering concurrent SSR renders that await the bridge,
pooled-engine cancellation, and the authorization matrix.
* Emit [Authorize] from the generator and demonstrate it in a sample
Completes descriptor-level authorization: SvelteRemoteGenerator now lifts
[Authorize]/[AllowAnonymous] off the service class and its methods into
the generated descriptor, matching what the reflection fallback already
read. Class-level requirements combine with method-level ones, and
[AllowAnonymous] on a method opts it out of the service policy.
The RemoteFunctions sample gains an AdminApi guarded by an 'admin' policy
with one [AllowAnonymous] method, plus ordinary cookie authentication so
the demo is runnable and copy-safe.
Tests: generator emission, and the full HTTP matrix (anonymous, wrong
role, right role, allow-anonymous opt-out) against the sample.
* Serialize long/ulong/decimal as strings so 64-bit values survive
JavaScript numbers are IEEE-754 doubles. A long above 2^53 or a decimal
fraction emitted as a JSON number is silently rounded in the browser -
9007199254740993 arrives as 9007199254740992, and money loses precision -
with no error on either side. The generated TypeScript said 'number',
so the compiler approved the corruption.
LargeNumberConverter writes these three as JSON strings and reads either
strings or numbers, so payloads from other clients still bind. All three
TypeScript emitters (metadata generator, remote generator, reflection
TypeGen) now say 'string' to match, including dictionary keys.
Also fixes enum-keyed dictionaries, which serialize fine but made type
generation throw. They now emit Partial<Record<Enum, T>> - an index
signature cannot be a union type.
The RemoteFunctions sample gains an AccountSummary(long, decimal, int)
showing all three cases, asserted end to end on the wire.
* Resolve generated TypeScript names so they cannot collide or shadow the DOM
Generated interfaces are ambient globals with unqualified short names. Two
CLR types called Item produced two 'interface Item' declarations in one
block (reflection) or a hard build failure with no escape hatch
(generated), and a model called File or Event silently replaced the DOM's
across the entire project.
Naming is now two-stage. Emitters produce a namespace-qualified
placeholder, because no generator can see the whole application and so
cannot know whether a short name is free. The scaffolder runs last, across
every application assembly, resolves each placeholder to the shortest
unambiguous name that does not shadow a built-in global, and substitutes
throughout the generated output. [SvelteType("Name")] overrides the choice
and is reserved first.
Also fixes a bug this made visible: [SvelteComponent] on a SveltePage means
'override the component path', but it was also registering the page as a
view model, so type collection descended through PageModel and emitted
HttpContext, ModelState, Endpoint and the rest of ASP.NET as ambient
globals. The RemoteFunctions sample's models.d.ts drops from 1129 lines to
43 - only its own models. Framework types are now excluded outright rather
than named, since they are never declared.
Tests: naming policy in isolation, plus collision, DOM-shadowing,
[SvelteType], and no-placeholder-leakage through the real scaffolder.
* Fix enum form binding, scalar JSON escaping, and async return unwrapping
Enum binding went through Enum.Parse, which accepts any numeric string:
a form field of "999" bound as (Priority)999 and reached the handler as a
value that does not exist. It also used .NET member names while JSON
binding used the camelCase names from JsonStringEnumConverter, so the same
parameter took different spellings depending on transport. Binding now
goes through the serializer first (with the CLR name still accepted for
hand-written forms) and rejects anything Enum.IsDefined does not know.
Scalar form values were wrapped in quotes by string concatenation, so any
field containing a quote, backslash or newline produced malformed JSON and
failed to bind for no visible reason. JsonSerializer.Serialize escapes it.
Reflection dispatch decided whether a handler returned a value by testing
the runtime type for the internal name "VoidTaskResult". It now uses the
method's declared return type, which is unambiguous - an async Task method
genuinely returns a Task<VoidTaskResult> at runtime, so no inspection of
the instance can tell the two apart without that BCL detail.
Tests cover every declared return shape through the reflection dispatcher,
plus enum spelling parity, undefined-enum rejection, and scalars
containing quotes and backslashes.
* Send the antiforgery token from enhance(), and close the CORS CSRF hole
SveltePage minted an antiforgery token into every data prop, the scaffolder
typed it, and the docs described it - but enhance() never sent it. Every
user had to hand-add a hidden __RequestVerificationToken input to every
form, which is exactly the boilerplate the helper exists to remove; the
TodoApp sample did it twice.
enhance() now sends the token as the RequestVerificationToken header.
AddSvelteNet configures AntiforgeryOptions.HeaderName when the app has not
chosen one, because ASP.NET sets none by default and would otherwise
reject the post. Forms that already render a hidden input keep working
untouched - the value is picked up automatically. The token may be a
getter, so a value taken from the data prop cannot go stale after an
enhanced response replaces it.
enhance() also ignores a re-entrant submit while one is in flight; a
double-click used to post the same mutation twice.
For remote functions, the X-SvelteNet header was the only CSRF defence,
and it holds only while no CORS policy lets a foreign origin send that
header - AllowAnyHeader() with AllowAnyOrigin() is common enough that this
was a real hole. Commands now also reject any request carrying a foreign
Origin. A request with no Origin is not from a browser and cannot be
CSRF'd, so server-to-server callers are unaffected.
New docs/security.md covers the authorization model, why it lives on the
descriptor rather than the endpoint, the CSRF checks, Jint's shared module
state, and loopback rendering. Added to the VitePress sidebar.
* Stop the scaffolder deleting user files, and make type generation incremental
The scaffolder unconditionally deleted Svelte/types.ts, Svelte/types.d.ts,
Svelte/routes.d.ts and per-page *.types.ts on every run. Those paths are in
the user's source directory; the deletes were leftover migration cleanup
from an older layout, so a hand-written file with one of those names was
destroyed silently on the next build. Only .svelte-net is cleared now.
Stale remote clients were found by scanning every *.remote.ts (and every
file literally named remote.ts) for the string "Generated by SvelteNet" and
deleting the matches - so a hand-written file that happened to contain that
line, such as one copied from generated output, was deleted too. The
scaffolder now records the files it writes and removes only those, which
also makes cleanup correct when a service is renamed or moved.
SvelteNetGenerateTypes had no Inputs/Outputs, so every single build spawned
a process, loaded the app assembly into an isolated ALC and rewrote the
generated files - which retriggers file watchers and made dotnet watch
loops needlessly expensive. It is now stamped against the built assembly:
verified to skip when nothing changed and to rerun when a source file does.
UseSvelteNet's startup scaffolding is now Development-only. It writes into
the content root, and an app rewriting its own source tree at boot on a
read-only or shared deployment is not something a stray configuration flag
should be able to cause; it logs a warning instead.
* Split SvelteNet.Jint into its own package and multi-target net8.0
SvelteNet.Core took a hard PackageReference on Jint, so every consumer
shipped a ~3 MB JavaScript engine - including client-only apps and apps
that had chosen the Node.js or Bun renderer. That contradicted the
"SSR is opt-in" design. JintSsrEngine, SsrModuleLoader and JintSsrOptions
now live in SvelteNet.Jint, which brings AddJintSSR with them;
SvelteNetBuilder exposes AddInProcessSsrFetchBridge() so any in-process
renderer can opt into direct query dispatch.
Everything shipped now targets net8.0 and net10.0. net8.0 is LTS and most
of the ASP.NET installed base; net10.0-only excluded them. The one .NET 9+
API in use (JsonStringEnumMemberNameAttribute) is behind a version check.
SvelteNet.Build targets net8.0 with RollForward=LatestMajor so a single
copy of the tool runs under whichever SDK builds the app.
The analyzer and build tool were packed from hardcoded
bin/$(Configuration)/<tfm> paths, which break under ArtifactsPath, a custom
BaseOutputPath, or any build-order variation - and break by silently
producing a package with no analyzer rather than failing. They are now
resolved from the referenced projects' real outputs, and the pack errors
out if either is missing.
CI gains a windows-latest matrix leg (path separators, process handling and
file-system case sensitivity all differ, and Windows is most of this
library's audience), installs the net8.0 runtime so the shipped TFM is
actually tested, and packs SvelteNet.Jint too.
* API and DX cleanups across the .NET and JavaScript sides
Registration:
- AddSvelteNet no longer infers dev mode from DOTNET_RUNNING_IN_CONTAINER.
Developing in a devcontainer silently disabled HMR with no way to tell
why. The hosting environment decides, falling back to the standard
environment variables when the environment is not on the collection yet.
- The GetCallingAssembly default is documented as the guess it is, and an
empty discovery scope now fails loudly instead of scanning nothing.
Options:
- ClientPublicPath is derived from ClientOutput minus WebRoot. They
described the same directory from two sides as independent defaults, so
changing one silently broke every asset URL. An explicit value still wins,
for CDN hosting.
JavaScript:
- The Vite plugin no longer mutates the caller's options object when
experimentalAsync is set.
- @sveltejs/vite-plugin-svelte moves to peerDependencies; as a hard
dependency it could install a second copy alongside the app's own.
- The "hydratable unavailable" warning fires once per page instead of once
per query per render, where it drowned out real console output.
- form() takes an `updates` callback to narrow which queries refresh after a
submit. The default refreshes everything, which is one request per query
on a large page; commands already had explicit .updates(...), so the two
now agree.
- readPageResponse's `response.ok !== false` is just `response.ok`.
* Sync docs, samples and roadmap with the remediation work
- docs/mvc.md moves to the async helpers and explains why the synchronous
overloads refuse when SSR is on.
- docs/getting-started.md notes that SveltePage prerenders in the page
filter, which is why the .cshtml accessors stay synchronous.
- docs/options.md documents generated type naming, [SvelteType], the derived
ClientPublicPath, and the extension-method caveat on discovery scope.
- docs/remote-functions.md documents form()'s updates callback.
- packages/sveltenet/README.md covers the antiforgery token and
double-submit protection.
- ROADMAP moves the shipped work across and records what the audit surfaced
but did not fix: a persistent SSR worker for Node/Bun, opt-in antiforgery
for remote functions, query cancellation, and the page-directory mapping
disagreement between the scaffolder and SveltePage.
- CLAUDE.md records the new invariants so they are not undone: async SSR,
descriptor-level authorization, two-stage type naming, the numerics
contract, multi-targeting, and that only .svelte-net may be deleted from.
* Fix code-review findings: inherited [Authorize], empty remote services, stamp path
Inherited [Authorize] was dropped on the generated path. The generator read
symbol.GetAttributes(), which returns only directly-applied attributes,
while AuthorizeAttribute is Inherited = true and both MVC and the
reflection fallback honour that. A service deriving from a secured base
produced a descriptor with no policy at all, so MapSvelteRemote attached no
metadata and the SSR bridge let the call through - on the path that is now
the default. The generator walks base types (and overridden methods), and
the reflection path is aligned to match subclasses of AuthorizeAttribute
rather than only the exact type.
A [SvelteRemote] class with no remote methods broke the consumer's build:
SvelteRemoteGenerator emits no dispatcher for it, but the metadata
generator still emitted a call to one (CS0103). That hits anyone who adds
the attribute before the first method, or comments the methods out.
The incremental build stamp landed in the project's source root, not obj/.
The targets file is imported from the csproj body, before the SDK defines
IntermediateOutputPath, so the property expanded to nothing - which had
already committed three stray stamp files in samples/. Resolved inside a
target instead, and the files are removed and ignored.
Also, all three pre-existing on this branch and found by the same review:
- Origin comparison no longer requires a matching scheme. Behind a
TLS-terminating proxy without UseForwardedHeaders, Request.Scheme is http
while the browser sends an https Origin, which rejected every POST from a
correctly deployed app. Host and port identify the origin.
- A command POST with no body is treated as no arguments. ContentLength is
null for chunked and HTTP/2 bodies, so it could not distinguish "no body"
from "streamed body" and answered 400.
- The Node/Bun fetch shim forwards all three HeadersInit forms. It used
Object.entries(), which yields nothing for a Headers instance or an array
of pairs, silently dropping the caller's headers.
Two further findings are recorded on the roadmap rather than patched: no-JS
form validation rendering problem JSON (needs errors carried across the
redirect) and the metadata generator holding Roslyn symbols in its
incremental pipeline (needs equatable value models).
Tests: 216 -> 222. The Node header test was verified to fail 2 of 3 cases
against the old shim, and the inherited-[Authorize] test against the old
generator.
* Regenerate the lockfile for the peer-dependency move
Moving @sveltejs/vite-plugin-svelte from dependencies to peerDependencies
changed the workspace manifest without updating bun.lock, so CI's
bun install --frozen-lockfile failed.
Also restores the em-dash in the package description, which a JSON rewrite
had escaped to —.
* Install chromium-headless-shell in CI
Vitest browser mode launches the headless shell, which Playwright ships as
a separate download from chromium. Installing only chromium left it absent
from the runner cache and every browser test failed to launch.
* Install Playwright browsers with the workspace's own playwright
bunx resolves and downloads its own copy of playwright, installs browsers
for that version, and rewrites bun.lock as a side effect. Vitest then
launches the pinned 1.61.1, whose chromium build was never downloaded, so
every browser test failed with 'Executable doesn't exist'.
Running the resolved binary from packages/sveltenet installs the browsers
the tests actually use.1 parent 0d4f6ea commit 30c0788
90 files changed
Lines changed: 3841 additions & 545 deletions
File tree
- .github/workflows
- docs
- .vitepress
- packages/sveltenet
- src
- test
- samples
- MvcHello
- Views/Hello
- RemoteFunctions
- Features/Admin
- TodoApp
- Svelte
- src
- SvelteNet.AspNetCore
- Dev
- Remote
- SvelteNet.Build
- SvelteNet.Core
- Remote
- TypeGen
- SvelteNet.FluentValidation
- SvelteNet.Generators
- SvelteNet.Jint
- tests
- SvelteNet.AspNetCore.Tests
- Fixtures
- SvelteNet.Core.Tests
- SvelteNet.Generators.Tests
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
14 | 20 | | |
15 | 21 | | |
16 | 22 | | |
17 | 23 | | |
18 | | - | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
19 | 29 | | |
20 | | - | |
| 30 | + | |
21 | 31 | | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
22 | 36 | | |
| 37 | + | |
| 38 | + | |
23 | 39 | | |
24 | 40 | | |
25 | 41 | | |
| |||
31 | 47 | | |
32 | 48 | | |
33 | 49 | | |
34 | | - | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
35 | 56 | | |
36 | 57 | | |
37 | 58 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
35 | | - | |
36 | | - | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
37 | 41 | | |
38 | 42 | | |
39 | 43 | | |
40 | 44 | | |
41 | 45 | | |
42 | 46 | | |
43 | | - | |
| 47 | + | |
44 | 48 | | |
45 | 49 | | |
46 | 50 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
7 | 12 | | |
| 13 | + | |
| 14 | + | |
8 | 15 | | |
9 | 16 | | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
10 | 37 | | |
11 | 38 | | |
12 | 39 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
| 48 | + | |
48 | 49 | | |
49 | 50 | | |
50 | 51 | | |
51 | 52 | | |
52 | 53 | | |
53 | | - | |
| 54 | + | |
54 | 55 | | |
| 56 | + | |
55 | 57 | | |
56 | 58 | | |
57 | 59 | | |
| |||
0 commit comments