Skip to content

Commit 08b0ec2

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 140f701 commit 08b0ec2

File tree

3 files changed

+32
-17
lines changed

3 files changed

+32
-17
lines changed

fixtures/fragments/file1.md

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

86-
## Local file with fragment
86+
## With fragment
8787

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

9191
[Link to local binary file with fragment](zero.bin#fragment)
92-
93-
## Remote URL with fragment
94-
95-
Right now, there is not MIME/content type based exclusion for fragment checks in the website checker.
96-
Also, other than the file checker, the website checker throws an error if reading the response body fails.
97-
The following link hence must fail:
98-
9992
[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
@@ -1889,9 +1889,9 @@ mod cli {
18891889
"https://raw.githubusercontent.com/lycheeverse/lychee/master/fixtures/fragments/zero.bin#fragment",
18901890
))
18911891
.stdout(contains("34 Total"))
1892-
.stdout(contains("28 OK"))
1892+
.stdout(contains("29 OK"))
18931893
// Failures because of missing fragments or failed binary body scan
1894-
.stdout(contains("6 Errors"));
1894+
.stdout(contains("5 Errors"));
18951895
}
18961896

18971897
#[test]

lychee-lib/src/checker/website.rs

Lines changed: 27 additions & 5 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,7 +9,7 @@ use crate::{
99
use async_trait::async_trait;
1010
use http::{Method, StatusCode};
1111
use octocrab::Octocrab;
12-
use reqwest::{Request, Response};
12+
use reqwest::{Request, Response, header::CONTENT_TYPE};
1313
use std::{collections::HashSet, time::Duration};
1414

1515
#[derive(Debug, Clone)]
@@ -107,8 +107,25 @@ impl WebsiteChecker {
107107
&& status.is_success()
108108
&& method == Method::GET
109109
&& response.url().fragment().is_some_and(|x| !x.is_empty())
110+
&& let Some(content_type) = response
111+
.headers()
112+
.get(CONTENT_TYPE)
113+
.and_then(|x| x.to_str().ok())
110114
{
111-
self.check_html_fragment(status, response).await
115+
if content_type.starts_with("text/html") {
116+
self.check_html_fragment(status, response, FileType::Html)
117+
.await
118+
} else if content_type.starts_with("text/markdown")
119+
|| (content_type.starts_with("text/plain")
120+
&& std::path::Path::new(response.url().path())
121+
.extension()
122+
.is_some_and(|x| x.eq_ignore_ascii_case("md")))
123+
{
124+
self.check_html_fragment(status, response, FileType::Markdown)
125+
.await
126+
} else {
127+
status
128+
}
112129
} else {
113130
status
114131
}
@@ -117,7 +134,12 @@ impl WebsiteChecker {
117134
}
118135
}
119136

120-
async fn check_html_fragment(&self, status: Status, response: Response) -> Status {
137+
async fn check_html_fragment(
138+
&self,
139+
status: Status,
140+
response: Response,
141+
file_type: FileType,
142+
) -> Status {
121143
let url = response.url().clone();
122144
match response.text().await {
123145
Ok(text) => {
@@ -126,7 +148,7 @@ impl WebsiteChecker {
126148
.check(
127149
FragmentInput {
128150
content: text,
129-
file_type: crate::FileType::Html,
151+
file_type,
130152
},
131153
&url,
132154
)

0 commit comments

Comments
 (0)