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.

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

fileserver: Add file_limit option for browse #6648

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

atakanyenel
Copy link

This PR fixes the issue #6644 . I'm not sure about the config name file_limit, I prefer max_dir_limit etc.The default is still 10000.

As mentioned in the issue, this PR gave me some background on file serve, so I will try to look at paging as well. Please feel free to edit any documentation as you see fit.

@CLAassistant
Copy link

CLAassistant commented Oct 20, 2024

CLA assistant check
All committers have signed the CLA.

modules/caddyhttp/fileserver/browse.go Outdated Show resolved Hide resolved
modules/caddyhttp/fileserver/browse.go Outdated Show resolved Hide resolved
modules/caddyhttp/fileserver/caddyfile.go Outdated Show resolved Hide resolved
@francislavoie francislavoie changed the title Add file_limit option for file_server browse fileserver: Add file_limit option for browse Oct 20, 2024
@francislavoie francislavoie added the feature ⚙️ New feature or request label Oct 20, 2024
Copy link
Member

@francislavoie francislavoie left a comment

Choose a reason for hiding this comment

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

Thanks!

For bonus points, it could probably use an adapt test (can just add it to an existing file_server test) to make sure it produces the correct JSON.

@mholt
Copy link
Member

mholt commented Oct 22, 2024

Thanks! This LGTM, but before I merge I have just one stupid nit: should it be called DirEntryLimit? It's a little more specific (could FileLimit be limiting file sizes? Something else?).

Similarly, defaultMaxDirLimit could be renamed defaultDirEntryLimit.

@atakanyenel
Copy link
Author

I added the test and updated the constant name. I'm keeping the config name still as "file_limit". I can also change that if required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants