Skip to content

Commit

Permalink
Fix asset URL paths
Browse files Browse the repository at this point in the history 
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).
  • Loading branch information
@nmattia
nmattia committed Oct 27, 2023
1 parent bcf5bfa commit bfbdb61
Show file tree
Hide file tree
Showing 4 changed files with 149 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
162 changes: 133 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,127 @@ 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, exp: Vec<String>) {
let mut exp = exp.clone();
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 bfbdb61

Please sign in to comment.