handle missing paths about silicon

[matt-42]

Indeed, the libcurl_json_client test fails because the procedure does not return a iod::sio object.
I'll update it to handle procedure returning file objects and let you know when you can test it.

[matt-42]

Hi zcourts,

For 1, I'm thinking on a patch that allows serving a directory. Today, it is not possible because all served routes have to be known in advance so if you serve /a, the route /a/b will show a 404.

There is one way we could change this, tell me what you think of this:

If our api serves the route /a, the first one is to match /a/b/c/d.html to the procedure under /a and let it handle the rest of the path accessible via a middleware, for example to serve the file b/c/d.html. If the file does not exist, throw error::not_found("your custom message")
(see here for example https://github.com/matt-42/silicon/blob/master/silicon/service.hh#L182 )

For 2, you can read backends/mhd.hh and try to write an equivalent for wangle. There is no reference yet on how to write a backend so if you have problems or questions, just ask it here.

[zcourts]

Hi Matthieu,

For 1, I think that's reasonable.
I think one way other frameworks get around this problem is by offering a combination of two things

1. your suggestion where the user can create a route /a and handle all /a/*.ext
2. offer a default middleware which serves static files from a known path like /static, in doing so the user only needs to enable the static file middleware but if they don't like /static or need to use a different path for any reason they can fallback to 1.

For 2, that's fine. I'll try to have a look into doing it this weekend.

[zcourts]

For what it's worth, I'm going to try to implement a middleware to do 2 from my suggestion above, serving /static, I'm happy to contribute that back upstream if you're interested. If you have any pointers, requirements for a PR let me know, otherwise I'll try to send a PR soon (latest next weekend if I don't get time before)

[matt-42]

Hi zcourts,

It's great if you can send this PR. I'll patch the router so that it let /a serve also /a/**/*.

For your PR, I suggest that you write the following file_server :

my_static_file_api = http_api(
   GET / _static = file_server("/path/to/the/directory/to/serve")
);

Note that this is not a silicon middleware. midlewares in silicon are entities that procedures can request as argument, (more on this here http://siliconframework.org/docs/middlewares.html). For example sqlite_connection :

 GET / _test = [] (sqlite_connection& c) {... }

file_server would be more a handler, as it serves a set of routes. Before you start, let's agree on how we'll implement it. This will be a function called everytime the server receive a request to serve a file. This function will basically take a mhd_request* r (we'll make it compatible with other backends once this work fine with MHD) and serve the file matching r->url (a std::string containing the full url) by returning a file object (https://github.com/matt-42/silicon/blob/master/silicon/file.hh) that the backend will stream back to the client.

file_server(mhd_request* r)
{
   string file_path;
   // Compute file_path from r->url

  // Return f, and let the backend stream it to the client.
  file(f);
}

We'll also have to handle procedure returning file object in the mhd_backend.

So to sumarize all the tasks:

• Update dynamic_routing_table.hh to let /a serve /a/**/* (matt-42)
• Write the file_server handler (zcourts)
• Update the MHD backend to stream file objects (matt-42).

Let me know if you still have questions or remarks.

[zcourts]

That sounds good. I'll have a go and send a PR hopefully by Weds if time allows. If not it'll be the weekend.

[zcourts]

Had a first look at this:
What flags do you build with, I can't compile the test dir (Mac, clang), see https://gist.github.com/zcourts/5c4bdb6b65b165630f16e8f5f78762fc

1. r->path is the "full URL" as in the entire path of the request right e.g. /a/some-file.txt
2. file_server handler gets added as file_server("/path/to/the/directory/to/serve"), but your signature in the example only has mhd_request* as a param. Where is /path/to/the/directory/to/serve stored (how do I access it)...or is r-url going to be prefixed with this so that it'll be /path/to/the/directory/to/serve/a/some-file.txt?
3. What computation is there to do on r-url, to me it seems like this: If r-url is prefixed with the base path thenr-url is already the full path, if it is not prefixed then concat base_path + r->url.
4. What behaviour do you want from file_server:
   1. should it perform checks on the file or is that something that'll be delegated to the back end?
   2. if file_path does not exist/can't be read? raise an exception?
   3. if file checks e.g. exist/readable are not being delegated to the backend, should I write those in file.hh so we can do file.exists() or file.readable() for e.g.

[matt-42]

Indeed, the last updates of the iod library broke the compilation of silicon. I fixed in my last commit your compilation problem and I'll fix the segv tonight.

Today r->url contains "http://domain.com/path/to/the/file.txt". So with GET / _a / _b = file_server("/serv/static") and "http://domain.com/a/b/c/d/file.txt" as a request url, you shoud delegate to the backend 'file('/serv/static/c/d/file.txt')'.

What I expect from file_server is to check if the request does not try to access a file outside the base directory for example "http://domain.com/a/b/../file.txt". You can delegate everything else to the backend.

I'll have time starting from tomorrow night to work on fixing silicon and the file serving. We should get something working by sunday night.

[matt-42]

I fixed the iod library. git update you https://github.com/matt-42/iod copy to compile and test your code.
I also updated the routing table to let us implement file_server 80549a8

[matt-42]

And file streaming : e66cf14
You now have everything to test the file_server procedure.

[matt-42]

Checkout this example, it may help: https://github.com/matt-42/silicon/blob/master/examples/file_streaming.cc

[zcourts]

That's good news. I'm out for the rest of the evening but I'll implement it in the morning.
One question is still outstanding.
If the user does this:

my_static_file_api = http_api(
   GET / _static = file_server("/path/to/the/directory/to/serve")
);

Unless I've missed something fundamental you're use of file_server procedure is ambiguous.
In the example above, base_path = /path/to/the/directory/to/serve, giving this signature file_server(const std::string& base_path).
But you also said that, as a handler, silicon will call expecting this signature file_server(mhd_request* r).

One of my questions above was, what happened to the base_path string?
When I'm implementing the checks on r->url, how do I know the base_path?
Are you adding that to the mhd_request* or making it available by some other means.

[matt-42]

You are right. I did a small mistake in my previous message. file_server("/path/to/the/directory/to/serve") shoud actually be a function that return the procedure serving the file :

inline auto file_server(std::string base_path)
{
  return [=path](mhd_request* r)
  {
     string file_path;
     // Compute file_path from r->url

    // Return file(f), and let the backend stream it to the client.
    return file(f);
  }
}

[matt-42]

Another mistake I made in my previous message : r->url does not contains "http://domain.com/a/b/c/d/file.txt" but "/a/b/c/d/file.txt" .

[zcourts]

I've raise the above PR for you to review.
I still can't compile even with the most recent iod updates.
I didn't get as much time as I thought I would, so I'll investigate the compile errors tomorrow so that I can get some tests written for it.

[matt-42]

You can use the libcurl_json_client to test it. There is one example here: https://github.com/matt-42/silicon/blob/master/tests/user_tracking.cc

[zcourts]

Okay, I couldn't figure out how to use the client to write the tests.
What an I doing wrong in the PR?

I tried to follow the examples you pointed at as well as the current tests but to no avail.

In file included from /Users/zcourts/projects/silicon/tests/static_file_server.cc:3:
In file included from /Users/zcourts/projects/silicon/tests/../silicon/backends/mhd.hh:21:
In file included from /Users/zcourts/projects/silicon/tests/../silicon/service.hh:8:
/Users/zcourts/projects/silicon/tests/../silicon/api.hh:59:16: error: no matching constructor for initialization of 'arguments_type' (aka 'iod::sio<>')
      : f_(f), default_args_(route.all_params()), route_(route)
               ^             ~~~~~~~~~~~~~~~~~~
/Users/zcourts/projects/silicon/tests/../silicon/api.hh:133:12: note: in instantiation of member function 'sl::procedure<sl::http_route<sl::http_get, std::__1::tuple<const s::_my_files_t, s::_my_file_t::variable_type<std::__1::basic_string<char>, iod::sio<> > >, iod::sio<s::_my_file_t::variable_type<std::__1::basic_string<char>, iod::sio<> > >, iod::sio<>, iod::sio<> >, iod::sio<>, sl::file, (lambda at /Users/zcourts/projects/silicon/tests/../silicon/backends/mhd.hh:631:12)>::procedure' requested here
    return procedure<Ro, A, Ret, F>(fun, route);
           ^
/usr/local/include/iod/sio.hh:118:10: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'sio<s::_my_file_t::variable_type<std::__1::basic_string<char>, iod::sio<> >>' to 'const sio<(no argument)>' for 1st argument
  struct sio<>
         ^
/usr/local/include/iod/sio.hh:118:10: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'sio<s::_my_file_t::variable_type<std::__1::basic_string<char>, iod::sio<> >>' to 'sio<(no argument)>' for 1st argument
/usr/local/include/iod/sio.hh:125:15: note: candidate constructor not viable: requires 0 arguments, but 1 was provided
    constexpr sio() { }
              ^

[matt-42]

I realized that we forgot to remove the path of the route.
In your test, you seek for the file ./files/my_files/hello.txt instead of ./files/hello.txt
I'll add a small middleware tonight for you to get the prefix path of the procedure.

[matt-42]

I added the prefix_path middleware, checkout this example: https://github.com/matt-42/silicon/blob/master/tests/prefix_path.cc and try to use it to remove the prefix from the file path computed by file_server

For the test, let's skip it for the moment as long as you can test it manually with curl. I'll add one when the libcurl client will be flexible enought.

[matt-42]

Hi zcourts,
Tell me if you have time to finalize the file_server by the end of this week.
If not, I'll finish it and write some doc so we can start using it next week.

[matt-42]

It is now working with 9ff9d78

Thanks @zcourts for this nice feature request and your contributions.

[zcourts]

Apologies for the delay. I went off on holiday and have only just caught back up with outstanding work. Good to see its all in. I'll start using it this week