Skip to content

Commit 063352f

Browse files
saeckielegaanz
authored andcommitted
Properly check link URLs
1 parent dc407d1 commit 063352f

3 files changed

Lines changed: 62 additions & 24 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,5 @@ tracing-subscriber = { version = "0.3.22", features = ["json", "env-filter"] }
3434
typst = "0.14.2"
3535
typst-assets = { version = "0.14.2", features = [ "fonts" ] }
3636
typst-eval = "0.14.2"
37+
url = "2.5.7"
3738
wasm-opt = "0.116.1"

src/check/readme.rs

Lines changed: 60 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use typst::{
1212
syntax::{FileId, VirtualPath},
1313
World,
1414
};
15+
use url::Url;
1516

1617
use crate::check::files;
1718
use crate::{
@@ -245,34 +246,69 @@ fn check_readme_link_url(
245246
diags: &mut Diagnostics,
246247
readme: &str,
247248
sourcepos: Sourcepos,
248-
url: &str,
249+
url_text: &str,
249250
) {
250-
let url = url.trim();
251+
let url_text = url_text.trim();
251252

252-
if url.contains("://") {
253-
// TODO: Should we check the URL here like for the `homepage` and
254-
// `repository` manifest fields?
253+
// Allow links that are only fragments: `#readme-section`.
254+
if url_text.starts_with("#") {
255+
// TODO: Consider validating that the fragment is valid.
256+
return;
257+
}
255258

256-
check_repo_file_url(diags, readme, sourcepos, url);
257-
} else if url.starts_with("#") {
258-
// TODO: Validate markdown anchor.
259-
} else {
260-
// Assume this URL is a path of a local file.
261-
if !files::path_relative_to(world.root(), Path::new(url)).exists() {
262-
diags.emit(
263-
Diagnostic::error()
264-
.with_code("readme/link/file-not-found")
265-
.with_message(format_args!(
266-
"Linked file not found: `{url}`.\n\n\
267-
Make sure to commit all linked files and possibly add them to the `exclude` list.\n\n\
268-
More details: https://github.com/typst/packages/blob/main/docs/tips.md#what-to-commit-what-to-exclude",
269-
))
270-
.with_labels(vec![Label::primary(
271-
readme_file_id(),
272-
sourcepos_to_range(readme, sourcepos),
273-
)]),
274-
);
259+
let url_error = match Url::parse(url_text) {
260+
Ok(url) => {
261+
// TODO: Should we check the URL here like for the `homepage` and
262+
// `repository` manifest fields?
263+
264+
check_repo_file_url(diags, readme, sourcepos, url.as_str());
265+
266+
return;
275267
}
268+
Err(error) => error,
269+
};
270+
271+
let invalid_url_error = || {
272+
Diagnostic::error()
273+
.with_code("readme/link/invalid-url")
274+
.with_message(format_args!("Invalid url: `{url_text}`\n{url_error}"))
275+
.with_labels(vec![Label::primary(
276+
readme_file_id(),
277+
sourcepos_to_range(readme, sourcepos),
278+
)])
279+
};
280+
281+
// The link couldn't be parsed as a URL, assume it's a local file.
282+
let file_url = format!("file:///{url_text}");
283+
let Ok(url) = Url::parse(&file_url) else {
284+
diags.emit(invalid_url_error());
285+
return;
286+
};
287+
288+
// Don't allow URL with empty paths. If the path consists only of the root
289+
// component that we added above, the path was completely empty before.
290+
let absolute_path = url.path();
291+
if absolute_path == "/" {
292+
diags.emit(invalid_url_error());
293+
return;
294+
}
295+
296+
// Check if the local file exists.
297+
if !files::path_relative_to(world.root(), Path::new(absolute_path)).exists() {
298+
diags.emit(
299+
Diagnostic::error()
300+
.with_code("readme/link/file-not-found")
301+
.with_message(format_args!(
302+
"Linked file not found: `{absolute_path}`.\n\n\
303+
Make sure to commit all linked files and possibly add them to the `exclude` list.\n\n\
304+
More details: https://github.com/typst/packages/blob/main/docs/tips.md#what-to-commit-what-to-exclude",
305+
))
306+
.with_note(format_args!("This link was assumed to be a local file because it's couldn't be parsed as an URL: `{url_text}`\n{url_error}"))
307+
.with_labels(vec![Label::primary(
308+
readme_file_id(),
309+
sourcepos_to_range(readme, sourcepos),
310+
)]),
311+
);
276312
}
277313
}
278314

0 commit comments

Comments
 (0)