Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow the routing methods to open a static file #63

Closed
sendilkumarn opened this issue Nov 25, 2018 · 15 comments
Closed

Allow the routing methods to open a static file #63

sendilkumarn opened this issue Nov 25, 2018 · 15 comments
Labels
design Open design question feature A feature that's ready for implementation

Comments

@sendilkumarn
Copy link

fn main() {
    let mut app = tide::App::new(());

   // Set static files context
    app.middleware(
          UrlPath::new()
            .prefix("/static/content/web/")
            .suffix(".html")
    );

  app.at("/").get(async || resource("index"));
  app.serve("127.0.0.1:7878");
}

P.S. Something similar to Rocket's NamedFile

@sendilkumarn sendilkumarn changed the title Allow the routing methods to open a static file available Allow the routing methods to open a static file Nov 25, 2018
@aturon aturon added design Open design question feature A feature that's ready for implementation labels Nov 27, 2018
@bIgBV
Copy link
Contributor

bIgBV commented Dec 4, 2018

Currently, middleware don't have any access to the Router, so this approach isn't possible. I think an approach that would work would be through configuration, as you would set up the static file serving up during server setup.

Looking at how it is done in rocket and actix, it seems that they are following something similar, both of them having internal configuration, rather than modifying shared configuration.

@DCjanus
Copy link
Contributor

DCjanus commented Dec 29, 2018

Maybe this feature could be implemented via Endpoint. Just like actix-web::fs::StaticFiles

    let mut app = tide::App::new(());
    app.at("/static/*").get(StaticFileEndpoint {
        root_path: PathBuf::from("./static/content/web"),
        suffix: ".html".to_string(), // maybe more configuration
    });

    let address = "127.0.0.1:8000".to_owned();
    println!("Server is listening on http://{}", address);
    app.serve(address)

@tomhoule
Copy link

I implemented something with an API in the same vein as what @DCjanus suggested. It is going to need a lot more configuration options, but it already works.

Here's the repo: https://github.com/tomhoule/tide-static-files

@DCjanus
Copy link
Contributor

DCjanus commented Mar 14, 2019

Great job for @tomhoule, but for prod, there is still a lot of other work to be done.

For example:

  • Range header(single range and multi range)
  • Content-Disposition header(Non-ASCII support?)
  • Better performance(less malloc or zero-copy by sendfile)

I was working on this too, but recently, I got a lot of job to be done, may not be able to complete this work.

For Range header parse, I wrote a simple libary via pest, might be help for you, @tomhoule

@tomhoule
Copy link

Thanks! Indeed, there's a lot of work ahead. I wanted to release early to get some feedback and not pour too much work into something that nobody will use. The three points you listed seem important, among others. Do you want to open issues for them? (otherwise I can do it of course).

@DCjanus
Copy link
Contributor

DCjanus commented Mar 14, 2019

@tomhoule My English is terrible, it's too hard to open such issue for me. 😭

@tomhoule
Copy link

tomhoule commented Mar 14, 2019

No worries, I can do it :) Feel free to open issues just to drop ideas without explaining at length, that helps a lot too.

edit: issues in other languages are ok too (I can read chinese)

@DCjanus
Copy link
Contributor

DCjanus commented Mar 15, 2019

I was going to develop such Endpoint in tide, so I forked tide and pushed some code to dcjanus/tide.

And then, inspired by tomhoule, separated repo might be better.

So I moved my code from dcjanus/tide to dcjanus/tide-static-file.

PS: new repo inspired a lot from @tomhoule

@tomhoule
Copy link

I think we should join efforts on one crate. @DCjanus has more features so we could use this crate as a starting point. I read the implementation and it looks like it's using blocking file operations without a thread pool, though, so that should probably be fixed.

@tomhoule
Copy link

I can contribute this part if we decide to standardize on this crate.

@DCjanus
Copy link
Contributor

DCjanus commented Mar 19, 2019

To be honest, for me, this repo is just for fun.
For me, it is very difficult to participate in this project steadily. I am willing to transfer this project to someone or contribute my code to @tomhoule's repo or add @tomhoule as collaborator.Maybe, participate in the development if I have time.
That's my bad to talk about my personal project in this issue, I am so sorry.

@tomhoule
Copy link

tomhoule commented Apr 5, 2019

With the latest changes in the revamp PR it became possible to make tide-static-files work again with the new abstractions.

@petejodo
Copy link
Collaborator

from what I understand, this issue is about serving a single file from within an endpoint, correct? Tide now supports serving static files under a directory...

app.at("/public/images").serve_dir("images/")?;

but maybe there's potentially a case where a user would actually want to handle this within an endpoint.

@mendelt
Copy link
Contributor

mendelt commented Dec 10, 2020

Isn't this covered with the serve_file method that was added recently?

@Fishrock123
Copy link
Member

Yeah I think this is probably covered by #725

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Open design question feature A feature that's ready for implementation
Projects
None yet
Development

No branches or pull requests

8 participants