Skip to content

Recent update to the filebrowser moduel breaks subdomains #399

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

Closed
evilhamsterman opened this issue Feb 8, 2025 · 6 comments · Fixed by #400
Closed

Recent update to the filebrowser moduel breaks subdomains #399

evilhamsterman opened this issue Feb 8, 2025 · 6 comments · Fixed by #400

Comments

@evilhamsterman
Copy link
Contributor

In the recent commit 22b2ad5 the command was changed to fix the fact that the filebrowser.db file didn't exist on first run so the filebrowser config set --baseurl command fails until the second time you run it.

However the way the command is interpreted if you use a subdomain and so therefore the baseurl is "" causes the command to fail because it thinks that there is no argument provided to the --baseurl option

$ cat /tmp/filebrowser.log
Error: flag needs an argument: --baseurl
Usage:
  filebrowser [flags]
  filebrowser [command]
...

Because of this it now fails to start at all when using a subdomain

@matifali
Copy link
Member

matifali commented Feb 8, 2025

Hi @dstoffel. I guess you should take a look.

@evilhamsterman thanks for the report amd sorry that the recent change broke it.

We probably need to have better integrated testing.

@dstoffel
Copy link
Contributor

sorry for the breaking change, i did not notice the tests file was not checking this.
@evilhamsterman let me know if you need help for the fix

@evilhamsterman
Copy link
Contributor Author

@dstoffel I think the issue can be resolved by just adding a = sign and quoting to ensure the --baseurl="${SERVER_BASE_URL}" is explicitly defined as an empty string if using a subdomain, which is valid. It just doesn't like no value being specified.

I created a PR as well that I thought might be a bit cleaner way of handling it long term, but both should work, which ever is easier to get merged and fix it

@lsalazarm99
Copy link

The fix isn't published yet, right? I'm still having this problem.

@matifali
Copy link
Member

Yes, we need to push an update. I will do so as soon as we merge #426

@matifali
Copy link
Member

Hi @lsalazarm99, this should be fixed in version 1.0.31. Can you test again? Thanks.

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 a pull request may close this issue.

4 participants