embedded-svc icon indicating copy to clipboard operation
embedded-svc copied to clipboard

Feature Request: Expand http server functions

Open Snapstromegon opened this issue 3 years ago • 4 comments
trafficstars

While creating an embedded-svc wifi manager I came across some issues which could easily be resolved by expanding the http server traits (the new ones) or adding new traits to the http server module.

These functions are:

  1. unregister handlers (esp-idf solves this via httpd_unregister_uri_handler and httpd_unregister_uri - I want to develop a captive portal and disable the routes for that when it's inactive
  2. registering an error handler (esp-idf solves this via httpd_register_err_handler) - this allows for handling custom 404 pages and similar
  3. expose the URI of a request (this was already discussed in esp-rs/esp-idf-svc#111)

Snapstromegon avatar Aug 11 '22 11:08 Snapstromegon

While creating an embedded-svc wifi manager I came across some issues which could easily be resolved by expanding the http server traits (the new ones) or adding new traits to the http server module.

These functions are:

  1. unregister handlers (esp-idf solves this via httpd_unregister_uri_handler and httpd_unregister_uri - I want to develop a captive portal and disable the routes for that when it's inactive

Thus is not possible without allocations. If you figure out a way - let me know. What I can do is implement a public, ESP-IDF specific method on EspServer which does not have a trait equivalent.

Btw, you can also always drop and recreate the whole HTTP server.

  1. registering an error handler (esp-idf solves this via httpd_register_err_handler) - this allows for handling custom 404 pages and similar

Let me think how to do this.

  1. expose the URI of a request (this was already discussed in Feature Request: Allow customization of httpd::Server configuration esp-idf-svc#111)

Already done with the new traits?

ivmarkov avatar Aug 11 '22 13:08 ivmarkov

While creating an embedded-svc wifi manager I came across some issues which could easily be resolved by expanding the http server traits (the new ones) or adding new traits to the http server module. These functions are:

  1. unregister handlers (esp-idf solves this via httpd_unregister_uri_handler and httpd_unregister_uri - I want to develop a captive portal and disable the routes for that when it's inactive

Thus is not possible without allocations. If you figure out a way - let me know. What I can do is implement a public, ESP-IDF specific method on EspServer which does not have a trait equivalent.

Btw, you can also always drop and recreate the whole HTTP server.

Hmm, I personally believe that every http implementation will need to have some kind of internal allocation so it can match routes at a later point for every request anyways, but I might be wrong here. Also this could be done in a seperate trait, so not every implementation has to implement this. But IMO it's also fine to ignore this case (for now) and make it a specific implementation or postpone it.

The trick with recreating is valid, but not always suitable for lib authors (which is my case), as a lib author will probably only want to dismiss the part of the server they are responsible for.

This is to me right now the least important of the three.

  1. registering an error handler (esp-idf solves this via httpd_register_err_handler) - this allows for handling custom 404 pages and similar

Let me think how to do this.

Personally I would offer at least an option for a generic 404 handler and one for internal server error. Alternatively it would be good to have either a

fn set_error_handler<H>(
    &mut self,
    err_status: u16,
    handler: H
)

or a more generic

fn set_error_handler<H>(
    &mut self,
    err_status_range: core::ops::Range
    handler: H
)

The first one allows to handle one specific error while the second one would allow to handle a whole range of error codes (like all 5XX errors).

  1. expose the URI of a request (this was already discussed in Feature Request: Allow customization of httpd::Server configuration esp-idf-svc#111)

Already done with the new traits?

I checked for this in the new http traits and neither in the embedded-svc not in the esp-idf-svc I can see a way to get the request uri.

Maybe I'm looking at the wrong place here?

Snapstromegon avatar Aug 11 '22 14:08 Snapstromegon

While creating an embedded-svc wifi manager I came across some issues which could easily be resolved by expanding the http server traits (the new ones) or adding new traits to the http server module. These functions are:

  1. unregister handlers (esp-idf solves this via httpd_unregister_uri_handler and httpd_unregister_uri - I want to develop a captive portal and disable the routes for that when it's inactive

Thus is not possible without allocations. If you figure out a way - let me know. What I can do is implement a public, ESP-IDF specific method on EspServer which does not have a trait equivalent. Btw, you can also always drop and recreate the whole HTTP server.

Hmm, I personally believe that every http implementation will need to have some kind of internal allocation so it can match routes at a later point for every request anyways, but I might be wrong here. Also this could be done in a seperate trait, so not every implementation has to implement this. But IMO it's also fine to ignore this case (for now) and make it a specific implementation or postpone it.

Look at edge-net as for how this can be done without alloc. (Still WIP though)

The trick with recreating is valid, but not always suitable for lib authors (which is my case), as a lib author will probably only want to dismiss the part of the server they are responsible for.

This is to me right now the least important of the three.

  1. registering an error handler (esp-idf solves this via httpd_register_err_handler) - this allows for handling custom 404 pages and similar

Let me think how to do this.

Personally I would offer at least an option for a generic 404 handler and one for internal server error. Alternatively it would be good to have either a

fn set_error_handler<H>(
    &mut self,
    err_status: u16,
    handler: H
)

or a more generic

fn set_error_handler<H>(
    &mut self,
    err_status_range: core::ops::Range
    handler: H
)

The first one allows to handle one specific error while the second one would allow to handle a whole range of error codes (like all 5XX errors).

  1. expose the URI of a request (this was already discussed in Feature Request: Allow customization of httpd::Server configuration esp-idf-svc#111)

Already done with the new traits?

I checked for this in the new http traits and neither in the embedded-svc not in the esp-idf-svc I can see a way to get the request uri.

Maybe I'm looking at the wrong place here?

The upcoming traits and their impls are in branches next, not in master anymore.

ivmarkov avatar Aug 11 '22 15:08 ivmarkov

Just a small note: I personally think that Query is not a good name for the trait containing uri and method information in the http context, as query has already a meaning in the context of an http request.

Sadly I don't have a better name alternative. I was thinking in the direction of RequestInfo, but every phrase was either not fitting or not specific enough.

Snapstromegon avatar Aug 11 '22 16:08 Snapstromegon

Most of this should be resolved with the new release of embedded-svc a couple of months ago. Feel free to open more specific bugs for anything you feel is not addressed yet.

ivmarkov avatar Jan 21 '23 15:01 ivmarkov