Embedio: Returning HTTP 405 on Incorrect Method

[shravan2x]

Consider this code:

AddHandler("/bob", HttpVerbs.Get, OnBob);

If a user sends a POST to /bob, a 404 Not Found is returned. This is generally the preferred behavior since most browsers don't support the 405 Method Not Allowed response well. But since it is technically the correct behavior (and REST APIs use it), could an option be added to enable returning 405s?

[mariodivece]

Yes, you have 2 options:

1. Write and register your own module returning the 405 response
2. Change the handler to accept all verbs and return a 405 when the verb is not HTTP GET

[geoperez]

@mariodivece we may add support to respond 405 to WebApiModulesince the developer doesn't have control over another method in a WebApiController. Many REST frameworks handle this scenario with 405.

Adding a fourth parameter to AddHandler (maybe optional to false) to allow the WebServerreturns 405, and change WebApiModuleto use AddHandler with the flag on.

[mariodivece]

@geoperez I would prefer that instead of adding a fourth parameter, we add a Func optional callback in the WebApi module that defaults to a prebuilt 405 response. I hope I made some sense.

[geoperez]

This is what we have right now: https://github.com/unosquare/embedio/tree/Issue124-ReturnHTTP405

[mariodivece]

I don't like it.

[FrankDoersam]

Okay and Why ?

Von meinem iPhone gesendet

Am 03.01.2018 um 22:31 schrieb Mario Di Vece [e-mail]:

I don't like it.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[mariodivece]

It is too specific... Are we going to start adding arguments to the handlers such as: willRespond200, willRespond205, willRespond999? This is terrible prectice. Furthermore, the response body of the 405 cannot be customized. Are we then going to add one more argument to the handler: string responseBody405 ? Just awful!

[geoperez]

@jpalcala can you check @mariodivece suggestions?

[FrankDoersam]

Okay perfekt 👍

Von meinem iPhone gesendet

Am 03.01.2018 um 22:35 schrieb Mario Di Vece [e-mail]:

It is too specific... Are we going to start adding arguments to the handlers such as: willRespond200, willRespond205, willRespond999? This is terrible prectice. Furthermore, the response body of the 405 cannot be customized. Are we then going to add one more argument to the handler: string responseBody405 ? Just awful!

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

[mariodivece]

Use overridable methods or callbacks please. Idea" overridable OnMethodNotAllowed -- Just a suggestion but I am busy having fun with ffmediaelement :)

[jpalcala]

Here's the PR https://github.com/unosquare/embedio/pull/130

[jpalcala]

@mariodivece what do you think ?

[geoperez]

I'm going to merge today this PR. Feedback is welcome.

[shravan2x]

Looks good to me!

[geoperez]

@shravan2x did you test the source code or you need a pre-release nuget?

[shravan2x]

@geoperez I've been experimenting with the code the last few days. Could you tell me why this doesn't return an HTTP 788 (some random number for testing) when a POST is made to /call?

```c#
public class Program : WebModuleBase
{
public string PreferredTitle => "EmbedIO Test Experiment";
public bool IsLongRunning => true;

public Program()
{
    AddHandler("/call", HttpVerbs.Get, SendRoot);
}

public static void Main(string[] args)
{
    using (WebServer server = new WebServer("http://127.0.0.1:17050/"))
    {
        server.RegisterModule(new Program());
        server.OnMethodNotAllowed += delegate (HttpListenerContext context)
        {
            context.Response.StatusCode = 788;
            return Task.FromResult(true);
        };
        server.RunAsync();

        while (true)
            Thread.Sleep(1000);
    }
}

public Task<bool> SendRoot(HttpListenerContext httpListenerContext, CancellationToken cancellationToken)
{
    Console.WriteLine($"Called at {DateTime.Now}");
    httpListenerContext.Response.KeepAlive = true;
    return Task.FromResult(true);
}

public override string Name => "Caller";

}
```

[geoperez]

We added the OnMethodNotAllowed execution only at WebApiModule.

You need to add a similar logic in your own module.

[shravan2x]

That behavior seems confusing since an exposed method does not work as expected. Can we either:

1. Change the OnMethodNotAllowed to protected so that users don't try to set it directly like I did above, or
2. Move the functionality to the WebApiModule class since it's only implemented there anyway?

[geoperez]

But the idea is to allow to the developer to change the OnMethodNotAllowed and use it on an own module. The same apply for OnNotFound.

The WebApiModule is using it for a better API-standard response.

[shravan2x]

I tried again, using a WebApiController this time, but still cannot get it to respond:

public class Program : WebApiController
{
    public string PreferredTitle => "EmbedIO Test Experiment";
    public bool IsLongRunning => true;

    public static void Main(string[] args)
    {
        using (WebServer server = new WebServer("http://127.0.0.1:17050/"))
        {
            server.WithWebApiController<Program>();
            server.OnMethodNotAllowed += delegate (HttpListenerContext context)
            {
                context.Response.StatusCode = 788;
                Console.WriteLine("hi");
                return Task.FromResult(true);
            };
            server.RunAsync();

            while (true)
                Thread.Sleep(1000);
        }
    }

    [WebApiHandler(HttpVerbs.Post, "/call")]
    public Task<bool> SendRoot(WebServer server, HttpListenerContext httpListenerContext)
    {
        Console.WriteLine($"Called at {DateTime.Now}");
        httpListenerContext.Response.KeepAlive = true;
        return Task.FromResult(true);
    }
}

It replies with a 405 Method Not Found instead of 788. Am I missing something again?

[geoperez]

@jpalcala can you take a look please?

[jpalcala]

Hi @shravan2x I used your example and noticed that you used += instead of = when using your custom OnMethodNotAllowed. Keep in mind that this will not overwrite the existing 405 response, It will execute both delegates. If I run your code I'll get a html response with 405 Method Not Found and a HTTP status code of 788. Just use = instead of +=.

[shravan2x]

@jpalcala That makes sense. Could we replace the OnMethodNotAllowed event with a virtual method instead? That should prevent this confusion in the future since only one handler can be registered.

EDIT: Nevermind, I just remembered that it's a property on the base class. I was able to get the code to work after the fix you suggested.

Just so I understand, the idea here is that users can register multiple event handlers and return false from the ones that don't handle it. Is this correct?

[geoperez]

OK, I'm going to release the nuget this week.

[jpalcala]

@shravan2x we kinda of use them for the sake of simplicity and maybe for dynamically changing the delegate. By the way a new EmbedIO version has been released.