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

Dream.static is returning incorrect headers for non-exists file #88

Closed
thangngoc89 opened this issue Jul 1, 2021 · 8 comments
Closed
Labels
Milestone

Comments

@thangngoc89
Copy link
Contributor

Given a static route like this one

Dream.get("/**") @@ Dream.static("public")

Try to access random route would have these behavior:

  • Chrome shows ERR_INVALID_RESPONSE
  • Safari downloads an empty file automatically
  • dream.log shows 404
  • curl -v -o - http://localhost:5000/not-exists
curl -v -o - http://localhost:5000/dasdabbbbsd
*   Trying ::1...
* TCP_NODELAY set
* Connection failed
* connect to ::1 port 5000 failed: Connection refused
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 5000 (#0)
> GET /dasdabbbbsd HTTP/1.1
> Host: localhost:5000
> User-Agent: curl/7.54.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Content-Type: application/octet-stream
< Set-Cookie: dream.session=0JDfhSEyb-zsmGQA1cFJae_G2; Max-Age=1209599; Path=/; HttpOnly; SameSite=Strict
< Content-Length: 0
<
* Connection #0 to host localhost left intact

@aantron said on Discord that those headers shouldn't be there.

  • Content-Type is probably what trigger safari to download it as file
@aantron aantron added this to the alpha3 milestone Jul 1, 2021
@aantron aantron closed this as completed in 809835d Jul 2, 2021
@aantron
Copy link
Owner

aantron commented Jul 2, 2021

Should be fixed by 809835d. The issue was that when the response is not one of the 2xx responses that carries content, Dream.static should not set the Content-Type based on the file extension, since file data is not being sent.

@thangngoc89
Copy link
Contributor Author

@aantron I've tested 809835d and it works perfectly. On safari it shows a blank page and I'm not sure if that's a normal behavior of safari because I can't think of any website without a 404 template

@aantron
Copy link
Owner

aantron commented Jul 2, 2021

Thanks!

Can you try comparing on Safari to 404 page from a Dream app without a 404 template, but outside /public? For example, compare with your own web app's general routes, or any of the examples, such as 3-router.

@thangngoc89
Copy link
Contributor Author

thangngoc89 commented Jul 2, 2021

@aantron it shows blank page as well. But I want to test non-dream behavior

@aantron
Copy link
Owner

aantron commented Jul 2, 2021

@aantron it shows blank page as well. But I want to test non-dream behavior

Understood. It was just a partial comparison you could do cheaply (since I don't have Safari) — thanks!

It's actually a bit disappointing that Safari has this behavior. Part of the reason I used blank replies by default is that Chrome's stand-in pages are localized by Chrome (at least on my system with my versions). So not using an error template actually helps with localization, while providing an English template would positively harm it.

Safari's blank page behavior undermines this pretty completely.

@aantron
Copy link
Owner

aantron commented Jul 2, 2021

while providing an English template would positively harm it.

That is, if Dream tried to provide its own default cookie-cutter templates, without knowing what the localization needs of each app are (if any).

@thangngoc89
Copy link
Contributor Author

@aantron oh. I don't suggest Dream would provide default error template.

Is there a way for me to provide a catch all hook for error pages?

@aantron
Copy link
Owner

aantron commented Jul 2, 2021

Is there a way for me to provide a catch all hook for error pages?

Absolutely, that's the intended way to do it, and Dream will even capture lower-level errors (so, all errors) and forward them to you hook. See the error example, also in the playground, and its API docs:

You can easily customize all error responses generated by Dream, whether from your application, or at lower levels in the HTTP stack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants