Several missing MIME type extension detections

[jbr]

these should be caught by the content sniffing, are they not?

[yoshuawuyts]

these should be caught by the content sniffing, are they not?

They indeed should be. Even .ico has a known magic byte definition.

@richardanaya are you noticing extensions are not correctly inferred? If so there might a deeper issue we should track down.

[richardanaya]

Hey all, I'm a noob tide user, let me show you the scenario I was writing that I noticed the breakage:

I appologize for this shitty Rust code. Basically I was writing a handler for a function that would check if a file exists, and if it existed, i'd returrn the file with correct Mime, but if it didn't exist, i'd just return "index.html"

app.at("/*").get(move |req: tide::Request<()>| {
                let server_dir3 = server_dir2.clone();
                async move {
                    let index = server_dir3.join("index.html");
                    let p = server_dir3.to_str().unwrap();
                    let p2 = req.url().path();
                    let s = format!("{}{}", p, p2).to_string();
                    let p3 = std::path::Path::new(&s);
                    if p3.exists() {
                        tide::Result::Ok(
                            tide::Response::builder(200)
                                .body(std::fs::read(p3).unwrap())
                                .content_type(
                                    from_extension(p3.extension().unwrap().to_str().unwrap())
                                        .unwrap(),
                                )
                                .build(),
                        )
                    } else {
                        tide::Result::Ok(
                            tide::Response::builder(200)
                                .body(std::fs::read(index).unwrap())
                                .content_type(tide::http::mime::HTML)
                                .build(),
                        )
                    }
                }
            });

I'd humbly offer to the convo if you all are making this function public, it should probably handle the cases it can from file extension so n00bs like me don't accidently wander into traps :P

[Fishrock123]

these should be caught by the content sniffing, are they not?

I don't think we should only sniff these. They are very common and this check is much cheaper.

[jbr]

@richardanaya I can see an argument against this function being public, as it is an http-types implementation detail. However, as a tide user, you should not have to write code like the above. Have you looked at serve_dir? Does it not meet your needs? How could we improve it so that you don't need to write your own file-serving endpoint?

[richardanaya]

@jbr thanks for your comments! I looked at serve_dir. I think part of me just didn't know this existed. The logic I was trying to implement fundamentally was:

1. listen to all paths /*
2. does this path exist realtive to a certain directory?
3. if I didn't exist, how do I provide a fallback index.html

This logic above mostly was to satisfy a single page app architecture.

I see now how serve dir could handle 1 & 2

app.at("/*").serve_dir("dist/")?;

not yet sure about #3 :) my assumption is a fallback route exists somewhere though

[jbr]

@richardanaya default not-found handling is a known issue with serve_dir. until we add a way to configure serve_dir, you probably want something like:

#[async_std::main]
async fn main() -> Result<(), std::io::Error> {
    tide::log::start();
    let mut app = tide::new();

    app.with(tide::utils::After(|response: tide::Response| async move {
        if response.status() == tide::StatusCode::NotFound {
            Ok(tide::Response::builder(200)
                .body(tide::Body::from_file("./dist/index.html").await.unwrap())
                .build())
        } else {
            Ok(response)
        }
    }));

    app.at("/").serve_dir("./dist")?;

    app.listen("127.0.0.1:8080").await?;

    Ok(())
}

[joshtriplett]

I'm going to close this issue, because all of these MIME types are currently handled correctly by content-sniffing.

Adding these extensions won't provide any performance improvement over content-sniffing, because we do content-sniffing first and only check extensions as a fallback. Changing that would be a much more substantial and breaking change, and would need consideration on its own merits.

If there's an issue with files not having their MIME types detected correctly, I'd suggest opening that as a separate issue.