- We were looking all total consumed bytes instead of bytes consumed since the last time we parsed a form value. This resulted in thinking that the key or value was too long if it couldn't be parsed after having parsed a bunch of data in the same read.
- Added tests
The only pubternal that remains is DfaGraphWriter - it's a good use case
for pubternal because it's something we want to expose, but we'd never
service and would willingly break the API in the future. It's also
really unlikly that anyone would build on top of it.
* Add a diagnostic source event that fires when a route is matched
- Usually more information becomes available about a request once route is matched. This event shoud allow diagnositc systems to enlighten the typical "begin request" metadata to include more information about the matched route and more importantly the selected endpoint and associated metadata.
* Update src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareTest.cs
Co-Authored-By: campersau <buchholz.bastian@googlemail.com>
* PR feedback and test fixes
* Added some doc comments to bedrock APIs
- Also cleaned up some HttpContext doc comments
* Apply suggestions from code review
PR feedback
Co-Authored-By: Andrew Stanton-Nurse <andrew@stanton-nurse.com>
* Apply suggestions from code review
Co-Authored-By: Justin Kotalik <jukotali@microsoft.com>
Adds infrastructure for a common IRouter-based pattern. In this pattern,
an extender subclasses Route to post-process the route values before MVC
action selection run. The new infrastructure duplicates this kind of
experience but based on endpoint routing.
The approach in this PR starts at the bottom... meaning that this is the
most-focused and least-invasive way to implement a feature like this.
Similar to fallback routing, this is a pattern built with matcher
policies and metadata rather than a built-in feature of routing.
It's valuable to point out that this approach uses IActionConstraint to
disambiguate between actions. The other way we could go would be to make
the *other* matcher policy implementations able to do this. This would
mean that whenever you have a dynamic endpoint, you will not by using
the DFA for features like HTTP methods. It also means that we need to go
re-implement a bunch of infrastructure.
This PR also adds the concept of an 'inert' endpoint - a non-Routable
endpoint that's created when fallback/dynamic is in use. This seems like
a cleaner design because we don't start *matching* RouteEndpoint
instances for URLs that don't match. This resolves#8130
* Read interface IList.Count once rather than per iteration
Inspired by #9962, read .Count once rather than once per loop iteration.
* Use nameof() instead of ToString()
Use constant nameof() on enum value, rather than runtime ToString().
* Right-size dictionary
Initialize dictionary with a fixed number of elements to the number of elements it contains.
* Use DisposeAsync()
Use DisposeAsync() on FileBufferingReadStream in input formatters.
* Override DisposeAsync()
Override DisposeAsync() to call DisposeAsync() on the inner stream.
* Use GetValueOrDefault() for content-length
Use GetValueOrDefault() to read the content length once instead of checking HasValue once and Value up to three times.
* Update Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs
Add DisposeAsync().
* Use DisposeAsync()
Use DisposeAsync() on transcoding streams as other formatters do.
- Remove string allocations caused by DiagnosticSource.Stop/StartActivity
- Pass the HttpContext directly as the object for StartActivity and StopActivity to avoid the anonymous object allocation.
- Though it's a bit ugly, added an HttpContext property to DefaultHttpContext to avoid breaking back-compat (which had to do reflection to get the HttpContext property anyways)
* Update build.cmd to install .NET Core into $repoRoot/.dotnet instead of $repoRoot/.dotnet/x64
* Move restore sources from build/sources.props into eng/Versions.props (following arcade conventions)
* Remove usages of RuntimeFrameworkVersion in tests and build
* Update Blazor VSIX to use Arcade VSIX tools
* Rename Common.Tests to IIS.Common.TestLib and make it a test asset
* Remove custom versions props for ANCM installer code
* Remove duplicate references to xunit and remove usages of IsTestProject
* Remove duplicate references to Internal.AspNetCore.Analyzers
* Import Arcade.Sdk props and targets and remove custom versioning props
* Remove references to Internal.AspNetCore.Sdk
* Rename PackageLicenseType => PackageLicenseExpression
* Remove dependency on tasks in Internal.AspNetCore.Sdk, add ref to Internal.AspNetCore.BuildTasks as a temporary workaround
* Use Arcade's nuspec support
* Rename SignalR.Client.FunctionalTests to SignalR.Client.FunctionalTestApp
* Fixes for changes to property evaluation order
* Update BaseLineGenerator to netcoreapp3.0
* React to changes in evaluation order in RPM files and quirks in using <Exec> instead of <Run>
* Update Microsoft.Extensions.ApiDescription.Server to react to changes in Arcade packaging
* Workaround aspnet/AspNetCore#11009
* EndpointMiddleware does not invoke endpoint
Fixes: #10649
The bug is that the endpoint delegate is not invoked when
SuppressCheckForUnhandledSecurityMetadata is set to true. This option is
provided so that can you suppress the security checks done by routing,
but right now what it does is suppress the entire middleware.
We had tests for the supression cases, but they didn't actually validate
that the middleware did any work, just that we don't throw. Fixed that.
* harden tests
The Arcade SDK requires that the obj/ and bin/ folders be placed in the top-level artifacts/ folder of the repo. Although this PR does not complete our Arcade convergence, this is a step towards updating our repo to build with the Arcade SDK.
Changes:
* Set output path for build to artifacts/bin/$(ProjectName)/
* Set intermediate output path for build to artifacts/obj/$(ProjectName)/
* Cleanup .gitignore files (remove duplication between repo-root and tested gitignore files)
* Add code check which looks for project files that share the same name (could cause issues)
* Rename project files to have unique names (avoid race condition of build output)
* Update all locations which were hard-coded to expect bin/ and obj/ in the project directory
* Add overrides for tests which still assert test binaries exist in a given location relative to the source code
- see #9923 for related discussion
- pin aspnet/EntityFrameworkCore dependencies
- chain aspnet/Extensions dependencies through aspnet/AspNetCore-Tooling instead
- run `darc` to grab the latest
- remove System.Text.Encodings.Web references
- now included in runtime.*.Microsoft.NETCORE.App packages
nits:
- copy a few `ReferenceResolution.md changes from aspnet/Extensions version
This changes the recipe for client-side blazor to use similar primitives
to server side applications.
---
I ignored auto-rebuild support because it's currently dead code until we
have that in VS. If we add auto-rebuild to ASP.NET Core - we'd probably want
to make that a separate gesture inside `IsDevelopement()` like other
dev-time features anyway.
---
The static files hookup is a special thing because creating the file
server for a client-side Blazor app involves some non-trivial work. We
plan to make this better in the future.
What's nice about this pattern is that the implementation is pretty
simple and literal, and it scales fine if you have multiple Blazor
client-side apps.
I didn't provide a lot of options here, it's pretty much the same as
UseBlazor.
---
I feel pretty good about the wireup with routing to use the
`index.html` from the client app. I think it's pretty to-the-point.
* Avoid buffering during input formatting for longer than necessary
EnableRewind uses FileBufferingReadStream which is not disposed until the response is completed.
This results in holding on to internal buffers for significantly longer than necessary. Changing it
to return the buffers immediately improved the allocations and throughput.
Adds functionality to parse a URI path given a way to
find an endpoint. This is the replacement for various machinications
using the global route collection and `RouteData.Routers` in earlier
versions.
For now I'm just adding a way to do this using Endpoint Name since it's
a pretty low level feature. Endpoint Name is also very direct, so it
feels good for something like this.
- This change tries to remove the EndpointSelectoContext allocation by making it a wrapper struct over the HttpContext. Unlike before, the HttpContext gets mutated once any component in the routnig pipeline sets a non null endpoint. This used to happen after the processing was complete.
- This change also implements the IRouteValuesFeature and IEndpointFeature in HttpProtocol to avoid the feature allocation and feature collection version churn.
- We also set the IRouteValuesFeature in IRouter based scenarios.
- Since we're not implementing IRoutingFeature in endpoint routing anymore
we can just create the RouteData inside MVC. We do this by polyfilling RouteData when using endpoint routing inside of MVC
- Implement GetRouteValue in terms of IRouteValuesFeature
- Noop if the EndpointRoutingMiddleware if an endpoint is already set
- Added tests
* Avoid allocations in more cases
Updates to DFA Matcher to avoid allocations in more cases. This makes
the matcher more pay-for-play.
- Avoid allocating an RVD while matching if possible
- Avoid allocating the candidate set unless necessary
First, avoid creating the RVD unless there are parameters or
constraints. This means that the candidate set can contain null route
value dictionaries. This seems fine because selectors are already
low-level. The route values feature will allocate an RVD when accessed,
so code in MVC or middleware won't even see a null RVD.
Secondly, avoid creating the candidate set unless there are selectors.
This will save an allocation most of the time since we won't need to run
selectors is really common cases. The candidate set is needed because
selectors mix mutability and async, so it needs to be a reference type.
However the default case is that we *don't* need to run selectors. The
impact of this is that we make a bunch of methods have an instance
variant and a static variant that operates on the array.
- This change tries to remove the EndpointSelectoContext allocation by making it a wrapper struct over the HttpContext. Unlike before, the HttpContext gets mutated once any component in the routnig pipeline sets a non null endpoint. This used to happen after the processing was complete.
- This change also implements the IRouteValuesFeature and IEndpointFeature in HttpProtocol to avoid the feature allocation and feature collection version churn.
* Update dependencies from https://github.com/dotnet/arcade build 20190423.2
- Microsoft.DotNet.GenAPI - 1.0.0-beta.19223.2
* Adjust to GenApi change in `enum` ordering
- was sorted by name, now by value
This is a crucial enabler for dynamic scenarios. A policy can replace an
endpoint with a *group* of dynamic endpoints which will be disambiguated
by other policies.
This change makes a bunch of random routing classes internal instead of
pubternal. Most of these have no use case and aren't valuable at all
outside of routing code.
The one exception here is that the only way to construct a
`TemplateBinder` in 1.0-2.2 is using a constructor that accepts some
pubternal types. I don't think it's a good idea to just yank this since
the usage is pretty wide.
Instead I added a factory service so you can create this, and marked the
constructor [Obsolete] where these types are touched. We can make the
actual breaking change in the next major release.
Use coventional routes for link generation
This change enables using conventional routes for link generation when
using MVC conventional routes. This change makes MVC link generation
behaviour highly compatible with 2.1.
The way that this works is that we create endpoints for **MATCHING**
using the denormalized conventional route, but we tell those endpoints
to suppress link generation.
For link generation we generate a non-matching endpoints per-route with
the same order value.
I added the concept of *required value any* to link generation. This is
needed because for an endpoint to participate in link generation using
RouteValuesAddress it needs to have some required values. These details
are a little fiddly, but I think it's worth doing this feature
completely.
* Make endpoint middleware explicit
This change makes the endpoint middleware explicit again, and updates
all of the templates.
The other change here is make UseEndpoints be the place where you
register endpoints. This is vital because it puts your code visually at
the point of the pipeline where it executes.
Lastly, I removed support for UseMvc with endpoint routing. This is
causing issues for some security features, and we're moving in the
direction of trying to make the middleware heavy implementation required
in 3.0. There are some issues we won't be able to fix in MVC if we can't
unambiguously know if UseMvc was used or the middleware.
Adds new constraints for checking if a route value is a file or not.
Added a new set of builder methods that specify what it means to be a
'fallback'. This is really similar to what the older SPA fallback routes
do, but this is lower in the stack and directly integrated with
endpoints.
This change introduces some new heuristics to make it easier to use
'page' or other resevered parameter names as parameters in URL
generation.
--
The main change here is to allow the link generation tree to *ignore* a
value passed in to URL generation when it conflicts with an endpoint's
required values.
The main concern of this feature area is "how do we tell whether you are
linking to an action or a page?". Routing attempts to do the right thing
will requiring very little from the user in terms of expressing intent.
In this case, we try to tell the difference between an attempt to generate
a link to an action due to the presence of the 'action' parameter and
absence of the 'page' parameter. This obviously doesn't work when you
want to use 'page' as a non-reserved parameter in an action. The same
case occurs for pages, but users are already used to the idea that
'action' is a reserved word in MVC.
We can loosen this restriction when the value that's supplied for 'page'
is known not to be any existing value of the 'page' route value.
This approach seems somewhat reasonable but has many of the problems inherent
to this area. When it fails (the value you want to use for 'page' causes
a conflict) - it's going to be esoteric and hard to understand.
Changes:
* Make Visual Studio 2019 a prerequisite for building this repo
* Update .sln files
* Update Windows SDK to 17134
* Update developer docs
* Disable ANCM tests
* Update to .NET Core SDK 3.0 Preview 2
* Use Microsoft.NET.Sdk.Razor as a package consistently accross the repo
* React to changes in metadata from Microsoft.NETCore.App
* React to changes in .NET Core SDK
* Attempt to workaround CodeCheck.ps1 failure which doesn't repro locally or on different agents. Possibly due to differences in the version of the PowerShell task?
* Remove dead YML file
* Rename usages of win7-{x64,x86} to win-{x64,x86}
* Update KoreBuild to 3.0.0-build-20190219.1
When a `<Reference>` is named `*.Sources`, set PrivateAssets=All (exclude from generated nuspec) and IncludeAssets=ContentFiles (only consume content files, not .dll's)
Changes:
* Add support for a property, `IsAspNetCoreApp`, in the .csproj file of assemblies which are part of the shared framework.
* Remove unused dependencies
* Remove reference which have become part of 'netcoreapp3.0'
This change enhances our ambient value logic to also deal with required
values. In 2.2 we introduced a 'required values' semantic to allow route
values to appear "to the left" of a route pattern for the purpose of
ambient values copying. This is a complicated way of saying "when you
like to a different endpoint then discard the ambient values".
What we didn't consider is that some ambient values are special (like
area). So basically, we'll allow an ambient value to be used if it's
part of the required values - even if we've already decided to discard
the ambient values.
This is a pretty surgical fix and only affected the desired scenario
based on tests.
-----
I also removed an optimization that I think is broken. I put an earlier
optimization in place that attempted to count ambient values as they
were "seen" to try and avoid some extra copying. This copying loop has a
cost even if it no-ops which is what I was trying to prevent.
Unfortunately since we added 'required values' - it's now possible for
an ambient value to be double-counted, which makes this optimization
incorrect.
- Use the array pool by default when the shared memory pool is specified for both the StreamPipeReader and StreamPipeWriter
- Support allocating unpooled memory if the StreamPipeWriter is asked for memory outside of the max pool size
- This should allow a more declarative approach to declaring endpoint metadata using the default methods.
- Attributes are applied first and can be overridden imperatively
* Fix#6102 - Intense CPU utilization on page change
The issue here was that every time a Razor Page changed, we would
subscribe an additional time to the endpoint change notifications. This
means that if you tweaked a page 30 times, we would update the address
table 31 times when you save the file. If you were doing a lot of editing
then this would grow to a really large amount of computation.
The fix is to use DataSourceDependentCache, which is an existing utility
type we developed for this purpose. I'm not sure why it wasn't being
used for this already. We're already using DataSourceDependentCache in a
bunch of other places, and it's well tested.
I also tweaked the stucture of this code to be more similar to
EndpointNameAddressScheme. This involved some test changes that all
seemed like good cleanup. The way this was being tested was a little
wonky.
(cherry picked from commit a5658a8c95)
* Fix#6102 - Intense CPU utilization on page change
The issue here was that every time a Razor Page changed, we would
subscribe an additional time to the endpoint change notifications. This
means that if you tweaked a page 30 times, we would update the address
table 31 times when you save the file. If you were doing a lot of editing
then this would grow to a really large amount of computation.
The fix is to use DataSourceDependentCache, which is an existing utility
type we developed for this purpose. I'm not sure why it wasn't being
used for this already. We're already using DataSourceDependentCache in a
bunch of other places, and it's well tested.
I also tweaked the stucture of this code to be more similar to
EndpointNameAddressScheme. This involved some test changes that all
seemed like good cleanup. The way this was being tested was a little
wonky.
- This completely removes the per request allocation until the feature is used.
- In order to make this change viable, we need to introduce a new HttpContextFactory that can accept new services without adding 2^n constructors. As a result, this change introduces a DefaultHttpContextFactory that takes an IServiceProvider and resolves dependencies based on the needs of the DefaultHttpContext and features.
- Throw in the older HttpContextFactory constructor when the IServiceScopeFactory is null
- It also saves us from revving the feature collection version unnecessarily.
John Luo
Ben Adams
Carlos J. Aliaga
David Fowler
Ryan Brandenburg
Ryan Nowak
dotnet-maestro[bot]
Ryan Nowak
Chris Ross
Hao Kung
Martin Costello
Javier Calvarro Nelson
Justin Kotalik
Günther Foidl
Kahbazi
Nate McMaster
Andrew Stanton-Nurse
huysentruitw
Doug Bunting
huysentruitw
Pranav K
James Newton-King
Gérald Barré
Bruno Costa
dotnet-maestro[bot]
BrennanConroy
Nathan
Pavel Krymets
JulijaRamoskiene
Joni
John Luo
Dávid Kaya