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

Response Content-Types #198

Closed
reenigneEsrever92 opened this issue Jul 17, 2021 · 10 comments · Fixed by #211
Closed

Response Content-Types #198

reenigneEsrever92 opened this issue Jul 17, 2021 · 10 comments · Fixed by #211
Labels
discussion This item needs some discussion

Comments

@reenigneEsrever92
Copy link

I've added a few static files to my project that I would like to serve with trunk.

Therefore I added them to my template file with rel="copy-dir" but they get served with the wrong mime-type.
Which leads to firefox rejecting them to due a failing sanitizer check.

Can we fix this? What would be the preferred strategy to do so?

@thedodd
Copy link
Member

thedodd commented Jul 21, 2021

Hey there. Thanks for the report. Do you mind me asking what type of files these are and what the expected MIME type is?

We are currently using async-std/tide for the server. Is this a know issue upstream?

@thedodd thedodd added the discussion This item needs some discussion label Jul 21, 2021
@reenigneEsrever92
Copy link
Author

reenigneEsrever92 commented Jul 24, 2021

Well, in case of font-awesome for example firefox sends this exact accept header: application/font-woff2;q=1.0,application/font-woff;q=0.9,/;q=0.8.

Firefox seems to not mind whether the file is woff2, woff or ttf, the header seems to always be the same.

Webpack for example (don't know the actual underlying implementation) doesn't include any content-type in its responses.

Trunk on the other hand serves them with content-type: text/html;charset=utf-8.

I've found the issue 27 on tide. Not quite sure as to how much this relates to the Problem though.

@lukechu10
Copy link
Contributor

Trunk on the other hand serves them with content-type: text/html;charset=utf-8.

When you look at the actual content, do you get the fonts or do you get the content of index.html? Trunk will serve index.html by default for non-existent paths.

@reenigneEsrever92
Copy link
Author

Ahh, ok. Thats actually the Problem, thanks. Maybe we could change that to reply with a 404 then?

@lukechu10
Copy link
Contributor

I think this is a duplicate of #192

@reenigneEsrever92
Copy link
Author

Makes sense

@WorldSEnder
Copy link

Since the other issue got closed, I suppose it makes sense to continue here. Would it be sensible to only fall back to serving index.html when the requested path doesn't end in a file extension? This should cover most needs I can see for SPAs and correctly return 404 for most non-existent resources.

@thedodd
Copy link
Member

thedodd commented Jul 28, 2021

@WorldSEnder that's not a bad idea. Perhaps we could actually evaluate the request's accept header to get a better idea on what is actually being requested. As @reenigneEsrever92 has pointed out, the browser is correctly passing along an accept with what appears to be accurate info. We could use that to more accurately drive our 404 logic. If the accept header does not include the pattern text/html somewhere within the value, and the requested resource does not exist, then we 404. That should give us a pretty damn high degree of reliability.

Thoughts?

@reenigneEsrever92
Copy link
Author

I find the idea very appealing. They are discussing content negotiation on the other thread there though maybe one could study that for a bit beforehand and/or even join in.

@thedodd
Copy link
Member

thedodd commented Jul 30, 2021

I'm nearly finished up with #202, and I decided to just go ahead and fix this issue as part of the updated. Seems to be working quite nicely now. Should have that code in master soon.

thedodd added a commit that referenced this issue Aug 4, 2021
Stable dist serving with proper index.html fallback after evaluating
accept header.

Add to changelog:
- [x] async everything in tools.
- [x] add a subcommand to clean cached tools (wasm-bindgen & wasm-opt).
- [x] resolves #198

TODO:
- [ ] ensure the shutdown broadcaster is closed as soon as a signal is
  received, else ^C too early will cause the signal to be missed and
  other parts of the app which have not initialized will not see the
  drop.
thedodd added a commit that referenced this issue Aug 4, 2021
Stable dist serving with proper index.html fallback after evaluating
accept header.

Add to changelog:
- [x] async everything in tools.
- [x] add a subcommand to clean cached tools (wasm-bindgen & wasm-opt).
- [x] resolves #198

TODO:
- [ ] ensure the shutdown broadcaster is closed as soon as a signal is
  received, else ^C too early will cause the signal to be missed and
  other parts of the app which have not initialized will not see the
  drop.
thedodd added a commit that referenced this issue Aug 4, 2021
Cut over to notify@0.5.0-pre.11. Much more simple interface, and works
quite nicely with the async ecosystem.

Cut over to axum. Much better!

The updated server stack is now more intelligently handling 404s. This
is a common issue where users may end up making a request for some
non-html static asset, but they get their path slightly wrong and are
served the index.html instead of a 404.

This typically results in users struggling to find the problem, and
often times users have opened issues in GH. This updated algorithm will
now evaluate 404s more closely and only respond with the index.html when
the `accept` header allows for text/html or */*.

Add to changelog:
- [x] all download utils have been make fully async.
- [x] added a subcommand to clean cached tools (wasm-bindgen & wasm-opt).
  This may be useful if there are ever issues with old tools which need
  to be removed.
- [x] resolves #198 (more
  intelligent 404s & index.html).

Todo:
- [ ] ensure the shutdown broadcaster is closed as soon as a signal is
  received, else ^C too early will cause the signal to be missed and
  other parts of the app which have not initialized will not see the
  drop.
thedodd added a commit that referenced this issue Aug 4, 2021
Cut over to notify@0.5.0-pre.11. Much more simple interface, and works
quite nicely with the async ecosystem.

Cut over to axum for web server. Much better!

The updated server stack is now more intelligently handling 404s. This
is a common issue where users may end up making a request for some
non-html static asset, but they get their path slightly wrong and are
served the index.html instead of a 404.

This typically results in users struggling to find the problem, and
often times users have opened issues in GH. This updated algorithm will
now evaluate 404s more closely and only respond with the index.html when
the `accept` header allows for text/html or */*.

Add to changelog:
- all download utils have been make fully async.
- added a subcommand to clean cached tools (wasm-bindgen & wasm-opt).
  This may be useful if there are ever issues with old tools which need
  to be removed.
- resolves #198 (more
  intelligent 404s & index.html).
- cut over to tokio 1.x.
- cut over to axum for web server.

Todo:
- [ ] ensure the shutdown broadcaster is closed as soon as a signal is
  received, else ^C too early will cause the signal to be missed and
  other parts of the app which have not initialized will not see the
  drop.
thedodd added a commit that referenced this issue Aug 4, 2021
Cut over to notify@0.5.0-pre.11. Much more simple interface, and works
quite nicely with the async ecosystem.

Cut over to axum for web server. Much better!

The updated server stack is now more intelligently handling 404s. This
is a common issue where users may end up making a request for some
non-html static asset, but they get their path slightly wrong and are
served the index.html instead of a 404.

This typically results in users struggling to find the problem, and
often times users have opened issues in GH. This updated algorithm will
now evaluate 404s more closely and only respond with the index.html when
the `accept` header allows for text/html or */*.

Added an example demonstrating all proxy functionality.

closes #198
closes #209
thedodd added a commit that referenced this issue Aug 4, 2021
Cut over to notify@0.5.0-pre.11. Much more simple interface, and works
quite nicely with the async ecosystem.

Cut over to axum for web server. Much better!

The updated server stack is now more intelligently handling 404s. This
is a common issue where users may end up making a request for some
non-html static asset, but they get their path slightly wrong and are
served the index.html instead of a 404.

This typically results in users struggling to find the problem, and
often times users have opened issues in GH. This updated algorithm will
now evaluate 404s more closely and only respond with the index.html when
the `accept` header allows for text/html or */*.

Added an example demonstrating all proxy functionality.

closes #198
closes #209
thedodd added a commit that referenced this issue Aug 4, 2021
Cut over to notify@0.5.0-pre.11. Much more simple interface, and works
quite nicely with the async ecosystem.

Cut over to axum for web server. Much better!

The updated server stack is now more intelligently handling 404s. This
is a common issue where users may end up making a request for some
non-html static asset, but they get their path slightly wrong and are
served the index.html instead of a 404.

This typically results in users struggling to find the problem, and
often times users have opened issues in GH. This updated algorithm will
now evaluate 404s more closely and only respond with the index.html when
the `accept` header allows for text/html or */*.

Added an example demonstrating all proxy functionality.

closes #198
closes #202
closes #209
thedodd added a commit that referenced this issue Aug 4, 2021
Cut over to notify@0.5.0-pre.11. Much more simple interface, and works
quite nicely with the async ecosystem.

Cut over to axum for web server. Much better!

The updated server stack is now more intelligently handling 404s. This
is a common issue where users may end up making a request for some
non-html static asset, but they get their path slightly wrong and are
served the index.html instead of a 404.

This typically results in users struggling to find the problem, and
often times users have opened issues in GH. This updated algorithm will
now evaluate 404s more closely and only respond with the index.html when
the `accept` header allows for text/html or */*.

Added an example demonstrating all proxy functionality.

closes #198
closes #202
closes #209
thedodd added a commit that referenced this issue Aug 5, 2021
Cut over to notify@0.5.0-pre.11. Much more simple interface, and works
quite nicely with the async ecosystem.

Cut over to axum for web server. Much better!

The updated server stack is now more intelligently handling 404s. This
is a common issue where users may end up making a request for some
non-html static asset, but they get their path slightly wrong and are
served the index.html instead of a 404.

This typically results in users struggling to find the problem, and
often times users have opened issues in GH. This updated algorithm will
now evaluate 404s more closely and only respond with the index.html when
the `accept` header allows for text/html or */*.

Added an example demonstrating all proxy functionality.

closes #198
closes #202
closes #209
thedodd added a commit that referenced this issue Aug 5, 2021
Cut over to notify@0.5.0-pre.11. Much more simple interface, and works
quite nicely with the async ecosystem.

Cut over to axum for web server. Much better!

The updated server stack is now more intelligently handling 404s. This
is a common issue where users may end up making a request for some
non-html static asset, but they get their path slightly wrong and are
served the index.html instead of a 404.

This typically results in users struggling to find the problem, and
often times users have opened issues in GH. This updated algorithm will
now evaluate 404s more closely and only respond with the index.html when
the `accept` header allows for text/html or */*.

Added an example demonstrating all proxy functionality.

closes #198
closes #202
closes #209
thedodd added a commit that referenced this issue Aug 5, 2021
Cut over to notify@0.5.0-pre.11. Much more simple interface, and works
quite nicely with the async ecosystem.

Cut over to axum for web server. Much better!

The updated server stack is now more intelligently handling 404s. This
is a common issue where users may end up making a request for some
non-html static asset, but they get their path slightly wrong and are
served the index.html instead of a 404.

This typically results in users struggling to find the problem, and
often times users have opened issues in GH. This updated algorithm will
now evaluate 404s more closely and only respond with the index.html when
the `accept` header allows for text/html or */*.

Added an example demonstrating all proxy functionality.

closes #198
closes #202
closes #209
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This item needs some discussion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants