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

fix(other): Fix null reference error when smtp account has no mailbox attached. #1451

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

Conversation

indridieinarsson
Copy link
Contributor

🍰 Pullrequest

Issue #1450 says it all. For some reason, I have an smtp account with no mailbox attached (I promise I didn't do anything funky with the config, all standard server configs through the web gui). This gives rise to a null reference error in the php code and an endless series of red "server error" warning dialogs in the web page.

Issues

#1450

@indridieinarsson indridieinarsson changed the title fix null reference error fix (backend) : Fix null reference error when smtp account has no mailbox attached. Feb 15, 2025
@indridieinarsson indridieinarsson changed the title fix (backend) : Fix null reference error when smtp account has no mailbox attached. fix(other) : Fix null reference error when smtp account has no mailbox attached. Feb 15, 2025
@indridieinarsson indridieinarsson changed the title fix(other) : Fix null reference error when smtp account has no mailbox attached. fix(other): Fix null reference error when smtp account has no mailbox attached. Feb 15, 2025
@Baraka24 Baraka24 requested a review from mercihabam February 18, 2025 08:55
@@ -1576,18 +1576,20 @@ public function process() {

foreach ($servers as $server_id => $config) {
$mailbox = new Hm_Mailbox($server_id, $this->user_config, $this->session, $config);
if ($mailbox->connect()) {
Copy link
Member

Choose a reason for hiding this comment

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

The connection object is not instantiated because the $config object lacks the type property, which the Hm_Mailbox class needs to determine the appropriate connection object to instantiate. Therefore, the fix should simply involve adding a type property to the $config object.

Copy link
Member

Choose a reason for hiding this comment

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

Right, but why is the config object missing a type - it should generally always be present...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mercihabam and @kroky : yes, right. There should be a type property there. But there isn't one. I have four accounts in my setup. 3 are "normal" imap (login with username and password) - they are ok. The fourth is a gmail account. That's the one where there is no 'type' property attached to. Can't really figure out why that is.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks @josaphatim . I'll take a look at this
I found out that the 'type' property is there at some point - but once we enter this code path (sending scheduled messages) it has disappeared. Only for the gmail account.
I strongly suspect it to have something to do with the oauth2 mechanism, since that's the only difference between the gmail account and the other 3.

@indridieinarsson
Copy link
Contributor Author

Second try.
Thanks @josaphatim . It seems the 'type' property wasn't being saved with the config to the database. For some reason, gmail accounts are affected while "normal" imap accounts are not - I think it has to do with the different control flow for xoauth authentication.
Anyways, I added 'type' to the nux/modules.php as suggested, removed the gmail account and added it again. Now it works.
I think it's worth checking with office365 and outlook accounts as well

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.

4 participants