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.

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

Fix asset URL paths #1987

Merged
merged 2 commits into from
Oct 30, 2023
Merged

Fix asset URL paths #1987

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bfbdb61
Fix asset URL paths
nmattia Oct 27, 2023
585cb14
Don't clone exp vector
nmattia Oct 30, 2023
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
15 changes: 15 additions & 0 deletions src/frontend/faq.html
View file
Open in desktop
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"
peterpeterparker marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

clone should not be necessary here

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you can get rid of it by declaring the argument mut:
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
Open in desktop
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