Skip to content

fix: resolve index file inside a directory#1752

Merged
mre merged 1 commit intolycheeverse:masterfrom
ocavue-forks:ocavue-fix-frg
Jul 3, 2025
Merged

fix: resolve index file inside a directory#1752
mre merged 1 commit intolycheeverse:masterfrom
ocavue-forks:ocavue-fix-frg

Conversation

@ocavue
Copy link
Contributor

@ocavue ocavue commented Jun 29, 2025

Closes #1751

This PR addresses issue #1751 by adding an "index" file check logic into Lychee. When the URL points to a local directory, it attempts to locate the index file within that directory and verify its fragments.

@ocavue ocavue marked this pull request as ready for review June 29, 2025 10:15
Comment on lines +115 to +119
// If path is a directory, and we cannot find an index file inside it,
// and we don't have a fragment, just return success. This is for
// backward compatibility.
else if path.is_dir() && !has_fragment {
return Status::Ok(StatusCode::OK);
Copy link
Contributor Author

@ocavue ocavue Jun 29, 2025

Choose a reason for hiding this comment

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

I added this logic to ensure that the following test can pass:

Skip the fragment check for directories like: [empty](empty_dir/).

While I personally prefer this test to fail, I may not be aware of all the context surrounding it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just found that I forgot to add the fragment to this test case. So it doesn't test the new code in #1713.

Instead, it has been treated as a success by lychee for a long time.

I'm not sure whether it's a common case for some scenarios or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kemingy Hey 👋

If I take out the quoted code in this comment, in other words, treat [empty](empty_dir/#no_exist_fragment) as an error, will this cause the "Too many open files" error to reappear? I'm not entirely clear on how checking link fragments relates to the "Too many open files" error.

Copy link
Contributor

Choose a reason for hiding this comment

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

No. I think it's safe to treat the empty_dir#fragment as an error.

Copy link
Contributor Author

@ocavue ocavue Jun 30, 2025

Choose a reason for hiding this comment

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

Nice! I'll keep this PR as it is to limit its scope. I'll open another PR to make empty_dir#fragment an error.

@mre
Copy link
Member

mre commented Jul 3, 2025

Fantastic. That should work!

@mre mre merged commit 6bcb37c into lycheeverse:master Jul 3, 2025
6 checks passed
@mre
Copy link
Member

mre commented Jul 3, 2025

I'll open another PR to make empty_dir#fragment an error.

Yes, much appreciated.

@mre mre mentioned this pull request Jul 3, 2025
@ocavue
Copy link
Contributor Author

ocavue commented Jul 4, 2025

I'll open another PR to make empty_dir#fragment an error.

Yes, much appreciated.

#1756

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect fragment check for index.html in a sub directory

3 participants