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.

Sign up for GitHub

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

Jump to bottom

fix(Mailer): Fix sendmail binary fallback #46480

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

fix(Mailer): Fix sendmail binary fallback #46480

wants to merge 5 commits into from
+8 −3

Conversation

joshtrichards
Copy link
Member

@joshtrichards joshtrichards commented Jul 12, 2024 •
edited
Loading

May fix #46457

Summary

The non-deprecated BinaryFinder returns false not null when the fallback is needed (versus the deprecated one in OC_Helper):

server/lib/private/BinaryFinder.php

Line 45 in 465ad2f

public function findBinaryPath(string $program) {

Also add debug logging of which binary is found and fix test.

Some historical context:

TODO

  • Make sure the tests are robust enough to catch this sort of thing

Checklist

@joshtrichards joshtrichards added bug 3. to review Waiting for reviews labels Jul 12, 2024
@joshtrichards joshtrichards added this to the Nextcloud 30 milestone Jul 12, 2024
@joshtrichards joshtrichards added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 12, 2024
@joshtrichards joshtrichards mentioned this pull request Jul 13, 2024
Closed
4 tasks
@joshtrichards
fix(Mailer): Fix sendmail binary fallback
51270fc 
Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards
test(Mailer): Use correct BinaryFinder
2fcbf11 
Signed-off-by: Josh Richards <josh.t.richards@gmail.com>
@joshtrichards
feat: add debug logging to sendmail binary finder
cbe57b5 
Signed-off-by: Josh Richards <josh.t.richards@gmail.com>
@joshtrichards joshtrichards force-pushed the fix/mailer-binaryfinder-fallback branch from a5711cb to cbe57b5 Compare July 14, 2024 11:52
@joshtrichards joshtrichards changed the title fix(Mailer): Adjust fallback for current BinaryFinder return values fix(Mailer): Fix sendmail binary fallback Jul 14, 2024
@joshtrichards joshtrichards mentioned this pull request Jul 14, 2024
Open
8 tasks
@joshtrichards
chore(debt): eliminate old BinaryFinder from settings
43bf52b 
Signed-off-by: Josh Richards <josh.t.richards@gmail.com>
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 14, 2024
View reviewed changes
apps/settings/lib/Settings/Admin/Mail.php
@@ -32,7 +32,7 @@
public function getForm() {
$parameters = [
// Mail
'sendmail_is_available' => (bool) \OC_Helper::findBinaryPath('sendmail'),
'sendmail_is_available' => (bool) \OCP\Server::get(IBinaryFinder::class)->findBinaryPath('sendmail'),

Check failure

Code scanning / Psalm

UndefinedClass

Class, interface or enum named OCA\Settings\Settings\Admin\IBinaryFinder does not exist
@joshtrichards
fix: add class / xxx squash this commit
dc5e118 
Signed-off-by: Josh Richards <josh.t.richards@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
2. developing Work in progress bug
Projects
None yet
Milestone
Nextcloud 30
Development

Successfully merging this pull request may close these issues.

[Bug]: nextcloud 29.0.3 mail_smtpmode sendmail doesn't fork /usr/sbin/sendmail
1 participant
@joshtrichards