Headers added in AfterRequest pipeline are ignored when handler returns 404 status

klym1 commented 5 years ago

Prerequisites

[x] I have written a descriptive issue title
[x] I have verified if the problem exist in both DEBUG and RELEASE mode
[x] I have verified that I am running the latest version of Nancy
[x] I have searched open and closed issues to ensure it has not already been reported

Description

When handler returns something with 404 status (NotFound) any custom headers added in pipelines.AfterRequest are ignored.

Steps to Reproduce

Enable custom headers in a bootstrapper as usual:

protected override void ApplicationStartup(TinyIoCContainer container, IPipelines pipelines)
{
    pipelines.AfterRequest.AddItemToEndOfPipeline(x =>
    {
         x.Response.WithHeader("Access-Control-Allow-Origin", "*");
         x.Response.WithHeader("Access-Control-Allow-Headers", "Authorization, Origin, Content-Type, Accept");
         x.Response.WithHeader("Access-Control-Allow-Methods", "GET, HEAD, POST, PUT, DELETE, OPTIONS, PATCH");
         x.Response.WithHeader("Access-Control-Expose-Headers", "Location, Content-Disposition, Content-Type");
    });
}

Create test handler:

public class TestModule : NancyModule
{
    public TestModule()
    {
        Get["/test"] = _ => HttpStatusCode.NotFound;
        Get["/test2"] = _ => HttpStatusCode.Forbidden;
    }
}

GET /test

Transfer-Encoding: chunked
Content-Type: application/json; charset=utf-8
Vary: Accept
Server: Microsoft-HTTPAPI/2.0
Date: Fri, 03 Aug 2018 12:07:14 GMT

GET /test2

Transfer-Encoding: chunked
Content-Type: text/html
Server: Microsoft-HTTPAPI/2.0
Access-Control-Allow-Origin: *
Access-Control-Allow-Headers: Authorization, Origin, Content-Type, Accept
Access-Control-Allow-Methods: GET, HEAD, POST, PUT, DELETE, OPTIONS, PATCH
Access-Control-Expose-Headers: Location, Content-Disposition, Content-Type
Date: Fri, 03 Aug 2018 12:07:57 GMT

GET /test should return 'Access-Control-*' headers as GET /test2 does

System Configuration

Nancy version:
- 1.4.4.0
Nancy host
- Nancy.Hosting.Self
Environment:
- Windows 10 x64
.NET Framework version:
- 4.7.1

klym1 commented 5 years ago

Guys, someone, anyone? This is a quite important question for me

ronnieoverby commented 5 years ago

Same thing for me. My cors headers are not in the 404 response.

cloudhunter89 commented 5 years ago

I've tried various approaches, but can not reproduce this issue. Using Nancyfx v1.4.4 and the below file code, I still get the cross origin headers

using Nancy; using Nancy.Bootstrapper; using Nancy.Hosting.Self; using Nancy.TinyIoc; using System;

namespace TestAfterPipeline { class Program { static void Main(string[] args) { HostConfiguration config = new HostConfiguration() { RewriteLocalhost = false, UrlReservations = new UrlReservations() { CreateAutomatically = false } }; INancyBootstrapper strapper = new SimpleBootStrapper();

  using (NancyHost host = new NancyHost(strapper, config, new Uri("

http://localhost:80"))) { host.Start(); Console.ReadLine(); host.Stop(); } } }

public class SimpleModule : NancyModule { public SimpleModule() { Get["400"] = _ => { return Nancy.HttpStatusCode.BadRequest; };

  Get["404"] = _ =>
  {
    return Nancy.HttpStatusCode.NotFound;
  };

  Get["500"] = _ =>
  {
    return Nancy.HttpStatusCode.InternalServerError;
  };
}

}

public class SimpleBootStrapper : DefaultNancyBootstrapper { protected override void ApplicationStartup(TinyIoCContainer container, IPipelines pipelines) { base.ApplicationStartup(container, pipelines); pipelines.AfterRequest.AddItemToEndOfPipeline(c => { c.Response.WithHeader("Access-Control-Allow-Origin", "*"); }); } } }

On Thu, Oct 11, 2018 at 9:21 PM Ronnie Overby notifications@github.com wrote:

Same thing for me. My cors headers are not in the 404 response.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/NancyFx/Nancy/issues/2920#issuecomment-429191626, or mute the thread https://github.com/notifications/unsubscribe-auth/AEEilkQYIhB9POHqJBoYEgKOPyneMJJGks5ukAq_gaJpZM4Vt4GP .

-- Jonathon Koyle

ronnieoverby commented 5 years ago

I will post repro.

ronnieoverby commented 5 years ago

Here's my bootstrapper:

public class BS : DefaultNancyBootstrapper
{
    protected override void ApplicationStartup(TinyIoCContainer container, IPipelines pipelines)
    {
        pipelines.AfterRequest += ctx =>
        {
            ctx.Response.WithHeader("Access-Control-Allow-Origin", "*")
                        .WithHeader("Access-Control-Allow-Methods", "POST, GET, DELETE, PUT, OPTIONS, PATCH")
                        .WithHeader("Access-Control-Allow-Headers", "Origin, X-Requested-With, Content-Type, Accept, Authorization")
                        .WithHeader("Access-Control-Max-Age", "3600");
        };
    }
}

I see a difference. pipelines.AfterRequest vs pipelines.AfterRequest.AddItemToEndOfPipeline. I can't test at the moment, but I'm assuming that would probably be the fix.

klym1 commented 5 years ago

So, I tried to use pipelines.AfterRequest instead of pipelines.AfterRequest.AddItemToEndOfPipeline, also I tried to call base method base.ApplicationStartup(container, pipelines); as @cloudhunter89 did, but with no apparent success. Here's full code, that I was using and which doesn't work for me:

    class Program
    {
        static void Main()
        {
            new NancyHost(new Uri("http://127.0.0.1:8000"), new Boot()).Start();
            Console.ReadKey();
        }
    }

    public class TestModule : NancyModule
    {
        public TestModule()
        {
            Get["/test"] = _ => HttpStatusCode.NotFound;
            Get["/test2"] = _ => HttpStatusCode.Forbidden;
        }
    }

    public class Boot : DefaultNancyBootstrapper
    {
        protected override void ApplicationStartup(TinyIoCContainer container, IPipelines pipelines)
        {
            base.ApplicationStartup(container, pipelines);

            pipelines.AfterRequest +=  x =>
            {
                x.Response.WithHeader("Access-Control-Allow-Origin", "*");
                x.Response.WithHeader("Access-Control-Allow-Headers", "Authorization, Origin, Content-Type, Accept");
                x.Response.WithHeader("Access-Control-Allow-Methods", "GET, HEAD, POST, PUT, DELETE, OPTIONS, PATCH");
                x.Response.WithHeader("Access-Control-Expose-Headers", "Location, Content-Disposition, Content-Type");
            };
        }
    }

UPD For the record - I was using Postman and Insomnia as REST clients

klym1 commented 5 years ago

@cloudhunter89 Can you please provide actual headers returned for each of your handlers? (400, 404, 500)

ronnieoverby commented 5 years ago

404

cloudhunter89 commented 5 years ago

Using your file (with my hostname instead of 127.0.0.1 which should not be a significant difference), I get the following:

$ curl -I http://c01622:8080/test
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                             Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0HTTP/1.1 404 Not Found
Transfer-Encoding: chunked
Content-Type: text/html
Server: Microsoft-HTTPAPI/2.0
Access-Control-Allow-Origin: *
Access-Control-Allow-Headers: Authorization, Origin, Content-Type, Accept
Access-Control-Allow-Methods: GET, HEAD, POST, PUT, DELETE, OPTIONS, PATCH
Access-Control-Expose-Headers: Location, Content-Disposition, Content-Type
Date: Tue, 16 Oct 2018 13:20:31 GMT

and

$ curl -I http://c01622:8080/test2
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                             Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0HTTP/1.1 403 Forbidden
Transfer-Encoding: chunked
Content-Type: text/html
Server: Microsoft-HTTPAPI/2.0
Access-Control-Allow-Origin: *
Access-Control-Allow-Headers: Authorization, Origin, Content-Type, Accept
Access-Control-Allow-Methods: GET, HEAD, POST, PUT, DELETE, OPTIONS, PATCH
Access-Control-Expose-Headers: Location, Content-Disposition, Content-Type
Date: Tue, 16 Oct 2018 13:27:20 GMT

However, if I hit my machine at the listening IP Address instead of the host name, I get the following:

$ curl -I http://192.168.56.1:8080/test
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                             Dload  Upload   Total   Spent    Left  Speed
  0   334    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0HTTP/1.1 400 Bad Request
Content-Length: 334
Content-Type: text/html; charset=us-ascii
Server: Microsoft-HTTPAPI/2.0
Date: Tue, 16 Oct 2018 13:20:58 GMT
Connection: close

kendolondon commented 5 years ago

I'm encountering the same issue - did you ever get a solution?

cloudhunter89 commented 5 years ago

@kendoglasgow Are you also using Postman to test the endpoint? I just tried it today and I see this:

kendolondon commented 5 years ago

Well that's bizarre! I'm seeing the same thing using Postman vs Chrome. It seems to be related to the request "Accept" header - I was able to make it work in Postman by changing the default "/" to the same one sent by Chrome. I then removed the header list items one by one till I found the one that breaks it - I ended up with "text/html,/" - so adding text/html to the Accept header list makes it work from Postman.

My ultimate problem, however, is that this issue is also causing my code to fail a test in a Python authored protocol test suite and I can't change the tests so any ideas what could be causing this on the Nancy side and how to fix it?

matias-quezada commented 4 years ago

1 year since this issue was open, no solution yet?

NancyFx / Nancy