add custom response support

[davidpeden3]

@Tratcher @mikaelm12

Addresses https://github.com/aspnet/BasicMiddleware/issues/135. Status reason/description are not implemented per https://github.com/aspnet/HttpAbstractions/issues/395.

Couple of notes:

I inlined the switch from UrlRewriteRuleBuilder.AddUrlAction. This cleaned up the actual logic flow and eliminated doing work for action types that didn't need it. This had the side effect that some of the enum parsing was moved inside of the try/catch block which caused inner exceptions to have duplicated line/column xml detail in the message. To solve this (and clean up the code a bit more), I created a dedicated exception for parse errors which allowed me not to double wrap some exceptions by filtering on the type.

I also added a test to assert that the action performs as expected.

[dnfclas]

Hi @davidpeden3, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

[Tratcher]

For full fidelity you can set the ReasonPhrase this way: https://github.com/aspnet/HttpAbstractions/issues/395#issuecomment-140153284

[davidpeden3]

yeah, i saw that. i'll add support for it tomorrow. it will require additional parsing.

[davidpeden3]

@Tratcher "full fidelity" would include status description which introduces a small wrinkle. it makes the stack go async to write the response. i can avoid that by calling .Wait which is probably ok since this is terminating the pipeline. so which option do you prefer?

1. partial fidelity (reason phrase but no description)

2. maintain sync:

    public class CustomResponseAction : UrlAction
    {
        public int StatusCode { get; }
        public string StatusReason { get; }
        public string StatusDescription { get; }

        public CustomResponseAction(int statusCode, string statusReason, string statusDescription)
        {
            StatusCode = statusCode;
            StatusReason = statusReason;
            StatusDescription = statusDescription;
        }

        public override void ApplyAction(RewriteContext context, BackReferenceCollection ruleBackReferences, BackReferenceCollection conditionBackReferences)
        {
            context.HttpContext.Response.StatusCode = StatusCode;
            context.HttpContext.Features.Get<IHttpResponseFeature>().ReasonPhrase = StatusReason;
            context.HttpContext.Response.WriteAsync(StatusDescription).Wait();
            context.Result = RuleResult.EndResponse;
            context.Logger?.CustomResponse(context.HttpContext.Request.GetEncodedUrl());
        }
    }

3. introduce async

    public class CustomResponseAction : UrlAction
    {
        public int StatusCode { get; }
        public string StatusReason { get; }
        public string StatusDescription { get; }

        public CustomResponseAction(int statusCode, string statusReason, string statusDescription)
        {
            StatusCode = statusCode;
            StatusReason = statusReason;
            StatusDescription = statusDescription;
        }

        public override async Task ApplyActionAsync(RewriteContext context, BackReferenceCollection ruleBackReferences, BackReferenceCollection conditionBackReferences)
        {
            context.HttpContext.Response.StatusCode = StatusCode;
            context.HttpContext.Features.Get<IHttpResponseFeature>().ReasonPhrase = StatusReason;
            await context.HttpContext.Response.WriteAsync(StatusDescription);
            context.Result = RuleResult.EndResponse;
            context.Logger?.CustomResponse(context.HttpContext.Request.GetEncodedUrl());
        }
    }

i have not assessed the extent to which async spreads.

[davidpeden3]

i went ahead with the sync version for now. will look at async if you think it's worthwhile.

[davidpeden3]

@mikaelm12 all feedback implemented

[davidpeden3]

@mikaelm12 i'm not sure why https://github.com/aspnet/BasicMiddleware/pull/200/commits/0eaf9b27924237afcd8fbd02c49058b03b6391ff failed the appveyor build. it was literally converting tabs to spaces.

[mikaelm12]

@davidpeden3 Looks like this needs to be rebased on dev. After that it should be good to go!

[davidpeden3]

@mikaelm12 I tried to install VS 2017 RC to keep up with https://github.com/aspnet/BasicMiddleware/commit/24a95f6c5dc7587fee24154f49bc8a42ddaafa3e and get all of my PRs up to date. Unfortunately, I've had a lot of issues getting it installed. I did manage to get it working yesterday but now I'm struggling with builds and tests. When i build aspnet/dev, I get a million build warnings like this:

2>C:\src\aspnet\BasicMiddleware\src\Microsoft.AspNetCore.HttpOverrides\obj\Microsoft.AspNetCore.HttpOverrides.csproj.nuget.g.props(19,5): warning MSB4011: "C:\Users\dpeden\.nuget\packages\internal.aspnetcore.sdk\1.0.1-rc2-15164\buildMultiTargeting\Internal.AspNetCore.Sdk.props" cannot be imported again. It was already imported at "C:\src\aspnet\BasicMiddleware\src\Microsoft.AspNetCore.HttpOverrides\obj\Microsoft.AspNetCore.HttpOverrides.csproj.nuget.g.props (16,5)". This is most likely a build authoring error. This subsequent import will be ignored. [C:\src\aspnet\BasicMiddleware\src\Microsoft.AspNetCore.HttpOverrides\Microsoft.AspNetCore.HttpOverrides.csproj]

and

6>C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(1912,5): warning MSB3277: Found conflicts between different versions of the same dependent assembly that could not be resolved.  These reference conflicts are listed in the build log when log verbosity is set to detailed.

Despite the warnings, I can successfully build the solution:

========== Rebuild All: 12 succeeded, 0 failed, 0 skipped ==========

Having said that, none of the tests show up in Test Explorer nor in ReSharper's test runner so something is definitely wrong. I'm not sure how to proceed.

[davidpeden3]

I blew everything away, re-cloned aspnet/dev and ran build.cmd. I am now in a working state in y'all's branch (though I still get warnings). I will try to get my branches updated and report back.

[davidpeden3]

I merged dev in. I cannot get tests to run. Code compiles. Will rely on your builds...

[Tratcher]

How about build.cmd from the command line? That should also run tests.

[davidpeden3]

Yeah, I had tried that (in my branch and it didn't work). Looking at the error, it was something related to a project.json file. It occurred to me that my branch must have still had some legacy stuff in it post-merge. I did a git clean -xdf as you had suggested a while back and then build.cmd worked as expected. Thanks!

[mikaelm12]

This needs to be rebased now that https://github.com/aspnet/BasicMiddleware/pull/168 was merged. Thanks for all the contributions!