Skip to content

fix: Set correct Content-Type when serving assets #136

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.

Sign up for GitHub

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

Closed
sbernauer wants to merge 1 commit into from
Closed

fix: Set correct Content-Type when serving assets #136

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions rust/stackable-cockpitd/src/handlers/ui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use axum::{
extract::Path,
http::{header::CONTENT_TYPE, HeaderValue},
response::{AppendHeaders, Html, IntoResponse},
http::{header::CONTENT_TYPE, HeaderMap, HeaderValue},
response::{Html, IntoResponse},
routing::get,
Router,
};
Expand All @@ -16,16 +16,20 @@ pub fn router() -> Router {
async fn ui() -> Html<&'static str> {
Html(stackable_cockpit_web::INDEX_HTML)
}
async fn asset(Path(name): Path<String>) -> impl IntoResponse {
(
AppendHeaders([(
CONTENT_TYPE,
match name.split_once('.') {
Some((_, "js")) => HeaderValue::from_static("text/javascript"),
Some((_, "css")) => HeaderValue::from_static("text/css"),
_ => HeaderValue::from_static("application/octet-stream"),
},
)]),
stackable_cockpit_web::ASSETS[&name],
)

/// Adds (or replaces) the Content-Type header with the type of the served asset.
/// So far only javascript and css are supported, for all the other types
/// `application/octet-stream` will be used.
Comment on lines +20 to +22
Copy link
Copy Markdown
Contributor

@nightkr nightkr Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a detail of the function, but not its primary purpose. This should be either a subsection of the doccomment, or a regular comment inside of the function.

async fn asset(mut headers: HeaderMap, Path(name): Path<String>) -> impl IntoResponse {
headers.insert(
Comment on lines +23 to +24
Copy link
Copy Markdown
Contributor

@nightkr nightkr Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't do what you want it to. The HeaderMap argument isn't some base set of headers defined by the framework, but the headers sent by the client. What this PR actually does is to reflect any headers sent by the user back at them.

Copy link
Copy Markdown
Contributor

@nightkr nightkr Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can test this yourself: curl 'http://localhost:8000/ui/assets/index-1272d2e9.js' -H 'foo: bar' -D- -o /dev/null | grep foo.

Copy link
Copy Markdown
Member Author

@sbernauer sbernauer Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Close enough ^^
Good catch, thx!

CONTENT_TYPE,
if name.ends_with(".js") {
HeaderValue::from_static("text/javascript")
} else if name.ends_with(".css") {
HeaderValue::from_static("text/css")
} else {
HeaderValue::from_static("application/octet-stream")
},
);
(headers, stackable_cockpit_web::ASSETS[&name])
}