Implement wildcard URI matching for http_server (IDFGH-10574)

[MightyPork]

This PR makes URIs in the HTTP server much more flexible, allowing to e.g. have optional trailing slashes, or to match all files and paths in a folder.

What has been done is best explained in the doc comment:

/**
 * @brief Test if a URI matches the given template.
 *
 * Template may end with "?" to make the previous character optional (typically a slash),
 * "*" for a wildcard match, and "?*" to make the previous character optional, and if present,
 * allow anything to follow.
 *
 * Example:
 *   - * matches everything
 *   - /foo/? matches /foo and /foo/
 *   - /foo/\* (sans the backslash) matches /foo/ and /foo/bar, but not /foo or /fo
 *   - /foo/?* or /foo/\*?  (sans the backslash) matches /foo/, /foo/bar, and also /foo, but not /foox or /fo
 *
 * The special characters "?" and "*" anywhere else in the template will be taken literally.
 *
 * @param[in] template - URI template (pattern)
 * @param[in] uri      - tested URI
 * @param[in] template - how many characters of the URI buffer to test
 *                       (there may be trailing query string etc.)
 *
 * @return true if a match was found
 */
static bool uri_matches(const char *template, const char *uri, const unsigned int len);

[MightyPork]

here is a tester I wrote to verify it's working correctly, for reference:

void main(void) 
{
  struct uritest {
    const char *template;
    const char *uri;
    bool matches;
  };
  
  struct uritest uris[] = {
    {"/", "/", true},
    {"", "", true},
    {"/", "", false},
    {"/wrong", "/", false},
    {"/", "/wrong", false},
    {"/asdfghjkl/qwertrtyyuiuioo", "/asdfghjkl/qwertrtyyuiuioo", true},
    {"/path", "/path", true},
    {"/path", "/path/", false},
    {"/path/", "/path", false},
        
    {"?", "", false}, // this is not valid, but should not crash
    {"?", "sfsdf", false},
    
    {"/path/?", "/pa", false},
    {"/path/?", "/path", true},
    {"/path/?", "/path/", true},
    {"/path/?", "/path/alalal", false},
    
    {"/path/*", "/path", false},
    {"/path/*", "/", false},
    {"/path/*", "/path/", true},
    {"/path/*", "/path/blabla", true},
    
    {"*", "", true},
    {"*", "/", true},
    {"*", "/aaa", true},
    
    {"/path/?*", "/pat", false},
    {"/path/?*", "/pathb", false},
    {"/path/?*", "/pathxx", false},
    {"/path/?*", "/pathblabla", false},
    {"/path/?*", "/path", true},
    {"/path/?*", "/path/", true},
    {"/path/?*", "/path/blabla", true},
        
    {"/path/*?", "/pat", false},
    {"/path/*?", "/pathb", false},
    {"/path/*?", "/pathxx", false},
    {"/path/*?", "/path", true},
    {"/path/*?", "/path/", true},
    {"/path/*?", "/path/blabla", true},
    
    {"/path/*/xxx", "/path/", false},
    {"/path/*/xxx", "/path/*/xxx", true},
    {}
  };
  
  struct uritest *ut = &uris[0];
  while(ut->template != 0) {
    
    bool match = uri_matches(ut->template, ut->uri, strlen(ut->uri));
    if (match != ut->matches) {
      printf("\x1b[31mERR\x1b[m should match? %d, matched? %d\x1b[m\r\n", 
             ut->matches, match);
    } else {
      printf("\x1b[32mOK\x1b[m matched? %d\x1b[m\r\n", 
             ut->matches);
    }
    
    ut++;
  }
  
  printf("END.\r\n");
}

[anurag-kar]

@MightyPork Thanks for this new feature. Please fix the conflict so that we can move forward.

[MightyPork]

sure i'll fix it. the change is small, so you could also just copy the new function and update the places where it's called from.

but didn't you want to merge #2578 before adding more stuff to the HTTP server?

[anurag-kar]

@MightyPork #2578 is almost through. It will get merged before this pull request is processed

[MightyPork]

the rebase resolved without any changes needed, it should be mergeable now

[olegantonyan]

Any ideas when this will be merged?

[MightyPork]

I have information they're a bit overloaded but working on it. So maybe we'll get it for Christmas For now you can copy the http server component to your project and apply this patch manually, that's what I've been doing for the last month or so. IDF gives it precedence over the library one if the name matches

…

On Tue, Nov 20, 2018, 4:47 PM Oleg Antonyan ***@***.*** wrote: Any ideas when this will be merged? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2581 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB8lHpPaO-mdKJcWIOXlLnR0WnlmUzouks5uxCQhgaJpZM4XnW_8> .

[olegantonyan]

Nice. Thanks for the great work

[anurag-kar]

@MightyPork Thank you for this extremely useful feature. Just to keep everyone updated about the status, we are in the process of reviewing this and hope to get this merged soon.

[MightyPork]

@anurag-kar what is the progress? I took a break to work on a different project, now came back to the one I developed this for, and sadly finding I still have copy and patch http_server myself :/

[anurag-kar]

@MightyPork The PR is still under review. We are considering the possibility of supporting the URI template RFC, in which case there may be additions on top of your feature. So expect some delay.

[MightyPork]

ok, just so you know why I added this, the main use case is static files. If you don't like my implementation, adding a simple "*" or NULL route acting as a catch-all would be sufficient for most use cases.
(Just a note - my wildcards are modelled after SpriteTM's old esphttpd library, which I used on the ESP8266 and missed it here. It also had Basic Auth and WebSockets - other great features that aren't here yet.)

Below is, for illustration, my server config that loads CSS, JS, favicon etc using the * route. I have a script that generates a look-up table, and rt_static_file() then looks for a file from the URI, or user_ctx if not NULL.

Without the wild-card, you would need to list every single file in the array here, and that's just annoying (plus there's more metadata to include, like mime type and file length, which just isn't where to put with one field void* user_ctx - or you would have to create one function per file, that's also not ideal).

I never heard of the "URI template RFC" before, so you get an idea how well known it is. Consider if it's in scope of the SDK to implement something like that - it can be easily done in user code after the route is matched by a wildcard.

static const httpd_uri_t routes[] = {
    {
        .uri = "/",
        .method = HTTP_GET,
        .handler = rt_static_file,
        .user_ctx = "A|index.html", // A| indicates login is needed
    },
    {
        .uri = "/login",
        .method = HTTP_GET,
        .handler = rt_static_file,
        .user_ctx = "login.html",
    },
    {
        .uri = "/login",
        .method = HTTP_POST, // post = form was submitted
        .handler = rt_login,
        .user_ctx = NULL,
    },
    {
        .uri = "/logout",
        .method = HTTP_GET,
        .handler = rt_logout,
        .user_ctx = NULL,
    },
    {
        .uri = "*", // needs wildcard routes
        .method = HTTP_GET,
        .handler = rt_static_file,
        .user_ctx = NULL,
    },
};

[olegantonyan]

+1 @MightyPork
I'm using it for static file too. No need for the full-featured uri templates, but simple catch-all handler would be just enough. IMO this is not "real" web application framework, you don't want Ruby on Rails in small MCU. But even if you do, than there should be a mechanism which allows you to implement you own custom routing.

[mahavirj]

@MightyPork Merged to ESP-IDF 91d6b3b. Please help to check once (there is also file serving example at https://github.com/espressif/esp-idf/tree/master/examples/protocols/http_server/file_serving).

[olegantonyan]

Doesn't work for me
This code https://github.com/olegantonyan/rmpd_player/blob/esp-idf-try-upgrade/firmware/main/web/web.c was working fine with old idf (november 2018) & patched esp_http_server. With new idf (a62cbfe) it produces "This URI doesn't exist" for all requests except statically defined (without wildcard)

[igrr]

In 416c55e, a new member was added to httpd_config structure, uri_match_fn. If you set it to httpd_uri_match_wildcard then matching will happen using wildcards. If it is NULL, then the matching will happen literally. You can also set it to a custom URI matching function. We have done this change on top the the current PR to ensure future extensibility, and to make sure wildcard handling would not break existing code.

[olegantonyan]

@igrr thank you! now it works

[MightyPork]

tested, works great! Also really like the way you made it extensible

[creatormir]

Below is, for illustration, my server config that loads CSS, JS, favicon etc using the * route. I have a script that generates a look-up table, and rt_static_file() then looks for a file from the URI, or user_ctx if not NULL.

Can you show an example of your implementation of the rt_static_file() function?