Skip to content

Commit a68c3ad

Browse files
committed
feat: skip fragment checking for unsupported MIME types
The remote URL/website checker currently passes all URLs with fragments to the fragment checker as HTML document, even if it is a different or unsupported MIME type. This can cause false fragment checking for Markdown documents, failures for other MIME types, especially binaries, and unnecessary traffic for large downloads, which are always finished completely, if the fragment checker is invoked. This commit checks the Content-Type header of the response: - Only if it is `text/html`, it is passed to the fragment checker as HTML type. - Only if it is `text/markdown`, of `text/plain` and URL path ends on `.md`, it is passed to the fragment checker as Markdown type. - In all other cases, the fragment checker is skipped and the HTTP status is returned. To invoke the fragment checker with a variable document type, a new `FileType` argument is added to the `check_html_fragment()` function. The fragment checker test and fixture are adjusted to match the expected result: checking a binary file via remote URL with fragment is now expected to succeed, since its Content-Type header does not invoke the fragment checker anymore. Signed-off-by: MichaIng <[email protected]>
1 parent 81f2605 commit a68c3ad

File tree

3 files changed

+38
-25
lines changed

3 files changed

+38
-25
lines changed

fixtures/fragments/file1.md

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,17 +104,10 @@ Even with fragment checking enabled, the following links must hence succeed:
104104
[Link to remote binary file without fragment](https://raw.githubusercontent.com/lycheeverse/lychee/master/fixtures/fragments/zero.bin)
105105
[Link to remote binary file with empty fragment](https://raw.githubusercontent.com/lycheeverse/lychee/master/fixtures/fragments/zero.bin#)
106106

107-
## Local file with fragment
107+
## With fragment
108108

109-
For local files URIs with fragment, the fragment checker is invoked and fails to read the content,
110-
but the file checker emits a warning only. The following link hence must succeed as well:
109+
Fragment checking is skipped if the Content-Type header is not "text/html", "text/markdown", or "text/plain" with ".md" URL path ending.
110+
Even that the URL contains a fragment, the following checks must hence succeed:
111111

112112
[Link to local binary file with fragment](zero.bin#fragment)
113-
114-
## Remote URL with fragment
115-
116-
Right now, there is not MIME/content type based exclusion for fragment checks in the website checker.
117-
Also, other than the file checker, the website checker throws an error if reading the response body fails.
118-
The following link hence must fail:
119-
120113
[Link to remote binary file with fragment](https://raw.githubusercontent.com/lycheeverse/lychee/master/fixtures/fragments/zero.bin#fragment)

lychee-bin/tests/cli.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1915,9 +1915,9 @@ mod cli {
19151915
"https://raw.githubusercontent.com/lycheeverse/lychee/master/fixtures/fragments/zero.bin#fragment",
19161916
))
19171917
.stdout(contains("42 Total"))
1918-
.stdout(contains("29 OK"))
1918+
.stdout(contains("30 OK"))
19191919
// Failures because of missing fragments or failed binary body scan
1920-
.stdout(contains("13 Errors"));
1920+
.stdout(contains("12 Errors"));
19211921
}
19221922

19231923
#[test]

lychee-lib/src/checker/website.rs

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::{
2-
BasicAuthCredentials, ErrorKind, Status, Uri,
2+
BasicAuthCredentials, ErrorKind, FileType, Status, Uri,
33
chain::{Chain, ChainResult, ClientRequestChains, Handler, RequestChain},
44
quirks::Quirks,
55
retry::RetryExt,
@@ -9,8 +9,8 @@ use crate::{
99
use async_trait::async_trait;
1010
use http::{Method, StatusCode};
1111
use octocrab::Octocrab;
12-
use reqwest::{Request, Response};
13-
use std::{collections::HashSet, time::Duration};
12+
use reqwest::{Request, Response, header::CONTENT_TYPE};
13+
use std::{collections::HashSet, path::Path, time::Duration};
1414

1515
#[derive(Debug, Clone)]
1616
pub(crate) struct WebsiteChecker {
@@ -108,7 +108,28 @@ impl WebsiteChecker {
108108
&& method == Method::GET
109109
&& response.url().fragment().is_some_and(|x| !x.is_empty())
110110
{
111-
self.check_html_fragment(status, response).await
111+
let Some(content_type) = response
112+
.headers()
113+
.get(CONTENT_TYPE)
114+
.and_then(|header| header.to_str().ok())
115+
else {
116+
return status;
117+
};
118+
119+
let file_type = match content_type {
120+
ct if ct.starts_with("text/html") => FileType::Html,
121+
ct if ct.starts_with("text/markdown") => FileType::Markdown,
122+
ct if ct.starts_with("text/plain") => {
123+
let path = Path::new(response.url().path());
124+
match path.extension() {
125+
Some(ext) if ext.eq_ignore_ascii_case("md") => FileType::Markdown,
126+
_ => return status,
127+
}
128+
}
129+
_ => return status,
130+
};
131+
132+
self.check_html_fragment(status, response, file_type).await
112133
} else {
113134
status
114135
}
@@ -117,19 +138,18 @@ impl WebsiteChecker {
117138
}
118139
}
119140

120-
async fn check_html_fragment(&self, status: Status, response: Response) -> Status {
141+
async fn check_html_fragment(
142+
&self,
143+
status: Status,
144+
response: Response,
145+
file_type: FileType,
146+
) -> Status {
121147
let url = response.url().clone();
122148
match response.text().await {
123-
Ok(text) => {
149+
Ok(content) => {
124150
match self
125151
.fragment_checker
126-
.check(
127-
FragmentInput {
128-
content: text,
129-
file_type: crate::FileType::Html,
130-
},
131-
&url,
132-
)
152+
.check(FragmentInput { content, file_type }, &url)
133153
.await
134154
{
135155
Ok(true) => status,

0 commit comments

Comments
 (0)