Skip to content

Commit

Permalink
Fix asset URL paths (#1987)
Browse files Browse the repository at this point in the history 
* Fix asset URL paths

This updates the logic for serving assets from the canister.

The logic previously did not cover all paths where an asset may be
found. For instance, an asset `/foo/index.html` may have returned 200 on
`/foo/` but 404 on `/foo`.

Moreover the `/faq` endpoint is fixed to actually return the expected
redirect to the FAQ. In practice the canister has extra logic for
handling `/faq`, but this fixes the HTML-redirect fallback. This ensures
that `/faq/` & `/faq/index.html` also redirect (which the canister does
not currently check for).

* Don't clone exp vector
  • Loading branch information
@nmattia
nmattia authored Oct 30, 2023
1 parent 8744d5b commit bf4ff08
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 37 deletions.
15 changes: 15 additions & 0 deletions src/frontend/faq.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta http-equiv="X-UA-Compatible" content="IE=edge" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<meta
http-equiv="Refresh"
content="0; url='https://identitysupport.dfinity.org/hc/en-us'"
/>
<title>Internet Identity</title>
<link rel="shortcut icon" href="/favicon.ico" />
</head>
<body></body>
</html>
8 changes: 0 additions & 8 deletions src/frontend/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,6 @@ const init = async () => {
// https://github.com/dfinity/internet-identity#build-features
showWarningIfNecessary();

// Redirect to the FAQ
// The canister should already be handling this with a 301 when serving "/faq", this is just a safety
// measure.
if (window.location.pathname === "/faq") {
const faqUrl = "https://identitysupport.dfinity.org/hc/en-us";
window.location.replace(faqUrl);
}

const okOrReason = await checkRequiredFeatures(url);
if (okOrReason !== true) {
return compatibilityNotice(okOrReason);
Expand Down
161 changes: 132 additions & 29 deletions src/internet_identity/src/assets.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,17 @@ fn collect_assets_from_dir(dir: &Dir) -> Vec<(String, Vec<u8>, ContentEncoding,
),
};

assets.push((file_to_asset_path(asset), content, encoding, content_type));
let urlpaths = filepath_to_urlpaths(asset.path().to_str().unwrap().to_string());
for urlpath in urlpaths {
// XXX: we clone the content for each asset instead of doing something smarter
// for simplicity & because the only assets that currently may be duplicated are
// small HTML files.
//
// XXX: the behavior is undefined for assets with overlapping URL paths (e.g. "foo.html" &
// "foo/index.html"). This assumes that the bundler creating the assets directory
// creates sensible assets.
assets.push((urlpath, content.clone(), encoding, content_type));
}
}
assets
}
Expand All @@ -283,33 +293,126 @@ fn file_extension<'a>(asset: &'a File) -> &'a str {
.1
}

/// Returns the asset path for a given file:
/// * make relative path absolute
/// * map **/index.html to **/
/// * map **/<foo>.html to **/foo
/// * map **/<foo>.js.gz to **/<foo>.js
fn file_to_asset_path(asset: &File) -> String {
// make path absolute
let mut file_path = "/".to_string() + asset.path().to_str().unwrap();

if file_path.ends_with("index.html") {
// drop index.html filename (i.e. maps **/index.html to **/)
file_path = file_path
.chars()
.take(file_path.len() - "index.html".len())
.collect()
} else if file_path.ends_with(".html") {
// drop .html file endings (i.e. maps **/<foo>.html to **/foo)
file_path = file_path
.chars()
.take(file_path.len() - ".html".len())
.collect()
} else if file_path.ends_with(".gz") {
// drop .gz for .foo.gz files (i.e. maps **/<foo>.js.gz to **/<foo>.js)
file_path = file_path
.chars()
.take(file_path.len() - ".gz".len())
.collect()
/// Returns the URL paths for a given asset filepath. For instance:
///
/// * "index.html" -> "/", "/index.html"
/// * "foo/bar.html" -> "/foo/bar", "/foo/bar/", "foo/bar/index.html"
///
/// NOTE: The behavior is undefined if the argument is NOT relative, i.e. if
/// the filepath has a leading slash.
///
/// NOTE: The returned paths will always start with a slash.
fn filepath_to_urlpaths(file_path: String) -> Vec<String> {
// Create paths, WITHOUT leading slash (leading lash is prepended later)
fn inner(elements: Vec<&str>, last: &str) -> Vec<String> {
if elements.is_empty() && last == "index.html" {
// The special case of the root index.html, which we serve
// on both "/" and "/index.html"
vec!["".to_string(), "index.html".to_string()]
} else if last == "index.html" {
// An index.html in a subpath
let page = elements.join("/").to_string();
vec![
format!("{page}"),
format!("{page}/"),
format!("{page}/index.html"),
]
} else if let Some(page) = last.strip_suffix(".html") {
// A (non-index) HTML page
let mut elements = elements.to_vec();
elements.push(page);
let page = elements.join("/").to_string();
vec![
format!("{page}"),
format!("{page}/"),
format!("{page}/index.html"),
]
} else if let Some(file) = last.strip_suffix(".gz") {
// A gzipped asset; remove suffix and retry
// XXX: this recursion is safe (i.e. not infinite) because
// we always reduce the argument (remove ".gz")
inner(elements, file)
} else {
// The default cases for any asset
// XXX: here we could create an iterator and `intersperse`
// the element but this feature is unstable at the time
// of writing: https://github.com/rust-lang/rust/issues/79524
let mut elements = elements.clone();
elements.push(last);
let asset = elements.join("/").to_string();
vec![asset]
}
}

let paths = match file_path.split('/').collect::<Vec<&str>>().split_last() {
None => {
// The argument was an empty string
// We can't really do much about this, so we fail explicitly
panic!("Expected non-empty filepath for asset");
}
Some((last, elements)) => inner(elements.to_vec(), last),
};

// Prefix everything with "/"
paths.into_iter().map(|path| format!("/{path}")).collect()
}

#[test]
fn test_filepath_urlpaths() {
fn assert_gen_paths(inp: String, mut exp: Vec<String>) {
exp.sort();

let mut actual = filepath_to_urlpaths(inp);
actual.sort();
assert_eq!(exp, actual);
}
file_path

assert_gen_paths(
"index.html".to_string(),
vec!["/".to_string(), "/index.html".to_string()],
);

assert_gen_paths(
"foo.html".to_string(),
vec![
"/foo".to_string(),
"/foo/".to_string(),
"/foo/index.html".to_string(),
],
);

assert_gen_paths(
"foo/index.html".to_string(),
vec![
"/foo".to_string(),
"/foo/".to_string(),
"/foo/index.html".to_string(),
],
);

assert_gen_paths("index.css".to_string(), vec!["/index.css".to_string()]);
assert_gen_paths("foo.bar.gz".to_string(), vec!["/foo.bar".to_string()]);

assert_gen_paths(
"sub/foo.bar.gz".to_string(),
vec!["/sub/foo.bar".to_string()],
);

assert_gen_paths(
"foo.html.gz".to_string(),
vec![
"/foo".to_string(),
"/foo/".to_string(),
"/foo/index.html".to_string(),
],
);

assert_gen_paths(
"sub/foo.html.gz".to_string(),
vec![
"/sub/foo".to_string(),
"/sub/foo/".to_string(),
"/sub/foo/index.html".to_string(),
],
);
}
1 change: 1 addition & 0 deletions vite.config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export default defineConfig(({ mode }: UserConfig): UserConfig => {
rollupOptions: {
// Bundle only english words in bip39.
external: /.*\/wordlists\/(?!english).*\.json/,
input: ["src/frontend/index.html", "src/frontend/faq.html"],
output: {
entryFileNames: `[name].js`,
// II canister only supports resources that contains a single dot in their filenames. qr-creator.js.gz = ok. qr-creator.min.js.gz not ok. qr-creator.es6.min.js.gz no ok.
Expand Down

0 comments on commit bf4ff08

Please sign in to comment.