-
-
Notifications
You must be signed in to change notification settings - Fork 756
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
SMTP: avoid loading templates if not enabled #1548
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: crabique <crabique@users.noreply.github.com>
Thanks for this PR and sorry for the delay. We have been working on our contribution guidelines and have delayed PR reviews for this reason. We now require a CLA. More details here |
CLA signed, thanks for the heads-up! |
Thanks for signing. I see you have used an email address like users.noreply.github.com. I think this is fine because we just need to make sure that you are entitled to provide a contribution, anyway I asked our lawyers to be sure (this is something I need to know in any case if it happens again in the future). They may be a little slow to respond, please be patient. Meanwhile I'll start taking a look at your PR. It looks good at first glance. Thank you! |
If you configure SMTP settings from the WebAdmin and try to reset a password SFTPGo will crash after this patch because templates are never loaded in this case. SMTP can be enabled from WebAdmin, so even if you start without configuration, email templates can be requested at runtime. Please explain your use case. If you want to execute SFTPGo as a single binary you build it embedding the templates and static resources. |
Hello! Sorry for the delay, didn't have much time for personal stuff. Thanks for checking this, I did not know it was possible to enable the feature at runtime from WebAdmin as I never actually used WebAdmin. My use case was that I just extracted a built binary from a tarball and wanted to spin up a minimal basic SFTP server without any bells and whistles, as I simply don't need anything besides the core SFTP file server. But my minimal config kept failing due to the missing templates, and even having the bundled templates feels wrong if they are not going to be ever used. A possible way to handle this would be to also check if WebAdmin is enabled, and if it's not, then there will be no way to re-enable SMTP at runtime, so the template loading could be safely skipped. But I understand it's not the cleanest way and my Go skill / SFTPGo architecture understanding is not good enough to offer a better fix at this time, so I'm completely fine if you close this PR, I tried and I failed 😄 |
91f38c6
to
0e77ba9
Compare
Hi!
This small fix aligns SMTP with the current
httpd
behaviour: at the moment, even ifsmtp.host
is''
, it still tries to load the templates before actually disabling SMTP capabilities.This change allows to
serve
without the templates/bundle if bothhttpd
andsmtp
are explicitly disabled.Checklist for Pull Requests