Static file MIME type inferred incorrectly for 404s

[ninjabear]

As I alluded to before without explaining properly, what I mean is it looks like the mime type is inferred from the file extension, but this should be overridden on certain events.

If you load up nickel-bootstrap and put in /images/afilethatdoesntexist.jpg instead of getting a 404 I get a firefox (YMMV) "image cannot be displayed because it contains errors" response.

Above is all speculation so far - it could be something else entirely. It could also be a hyper issue, depending on where the handover is.

Interestingly this doesn't occur in chrome, which displays the correct "file not found" response.

[Ryman]

Okay, it's 2 issues overlapping:

First, #163, mybad.

Secondly, The route "**" should match anything, but it's not doing so in this case and falling back to the default 404 handler instead. This is due to the file extension, which I fixed a long time ago, and when checking the code... it had vanished... so it looks like #91 was closed rather than merged accidentally and it wasn't noticed >_<

I'll look at rebasing that soon if noone else beats me to it!

[Ryman]

@ninjabear Can you confirm this is fixed?

[ninjabear]

@Ryman hum, I can't really test at the moment, not sure if its the serve static directories fix or something else but nickel-bootstrap just hangs on everything, will have to look properly later

[SimonTeixidor]

I'm seeing this issue. If I don't set the content type manually when responding with response.send() the response doesn't get a content type:

HTTP/1.1 200 OK
Date: Tue, 10 Mar 2015 12:51:28 GMT
Server: Nickel
Transfer-Encoding: chunked

Lalalalala

Firefox then assumes the response is an image if the requested url ends with .jpg and you get the "image cannot be displayed because it contains errors" message in the browser. My workaround for now is to set the response type manually when sending text on requests that could be an image.

[ninjabear]

I looked into it a bit further and my problem could be related to multiple gets with a keep-alive, as all the URLs work individually but not the landing page at once.

[Ryman]

@simonpersson Got it, we should set default mime to PlainTxt or Html even for those handlers (I think I removed it at one point when implementing the ResponseFinalizers).

@ninjabear The keepalive issue may be related to hyperium/hyper#368 ?

[SimonTeixidor]

Yeah, a default mime should do it.

I think there might be something else broken too. This code just causes a deadlock if path is not valid:

let response = try!(res.send_file(&*path));
Ok(Halt(response))

GDB tells me that it hangs on a thread.join() somewhere. Nothing is received on the client side, not even headers. I'm not sure that is related to the hyper bug, although I really don't know :)

[ninjabear]

@Ryman that looks like it was it. Cargo clean/update has returned it to working state. The MIME type for invalid files is also correct now, thanks.

One small note about #163 which is more an observation than a problem is that "**" doesn't match all routes where a file is not present in the static file handlers. It matches virtually all of them, but ones with [;!^&] (some other special characters probably) are still returning 404s. It could be that when checking if the path exists, it's not being checked if it's valid (for obvious reasons it should filter out ../../../bla etc anyway).

Starting to feel bad about raising so many niggles but that's all they are! In general I think the hyper conversion is a great success. I'm also open to doing any donkey tickets/simple jobs to help out along the way.

[ninjabear]

To repeat load up a nickel instance with static file handlers and a "**" route set (I used nickel-bootstrap) then

• visit "http://server/images/a.jpg" (works fine, directed to "**" route)
• visit "http://server/images/a!.jpg" (404, expected "**" route)

[Ryman]

@ninjabear Constructive feedback is always welcome! See my comments on #91 about moving to a blacklist, I still think that's better behavior to have (it was merged before we actually addressed the comment). Feel free to open an issue to change that and see if we can reach a consensus there!

@simonpersson Hmm, this is problematic, I know whats causing it, but I'm not sure of an elegant fix, I'll patch send_file for now and possibly one or two others, but I think the interface will need another iteration here!

[Ryman]

Should be closed by #170