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

Skip users without a valid email address #551

Merged
merged 1 commit into from
Oct 25, 2022

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Oct 24, 2022

Otherwise calling setTo may fail:

{
  "reqId": "",
  "level": 3,
  "time": "",
  "remoteAddr": "",
  "user": "--",
  "app": "PHP",
  "method": "",
  "url": "--",
  "message": "Undefined offset: 1 at /var/www/html/lib/private/Mail/Message.php#86",
  "userAgent": "--",
  "version": "23.0.9.1",
  "exception": {
    "Exception": "Error",
    "Message": "Undefined offset: 1 at /var/www/html/lib/private/Mail/Message.php#86",
    "Code": 0,
    "Trace": [
      {
        "file": "/var/www/html/lib/private/Mail/Message.php",
        "line": 86,
        "function": "onError",
        "class": "OC\\Log\\ErrorHandler",
        "type": "::"
      },
      {
        "file": "/var/www/html/lib/private/Mail/Message.php",
        "line": 148,
        "function": "convertAddresses",
        "class": "OC\\Mail\\Message",
        "type": "->"
      },
      {
        "file": "/var/www/html/apps/announcementcenter/lib/BackgroundJob.php",
        "line": 205,
        "function": "setTo",
        "class": "OC\\Mail\\Message",
        "type": "->"
      },
      {
        "file": "/var/www/html/lib/private/User/Manager.php",
        "line": 656,
        "function": "OCA\\AnnouncementCenter\\{closure}",
        "class": "OCA\\AnnouncementCenter\\BackgroundJob",
        "type": "->",
        "args": [
          "*** sensitive parameters replaced ***"
        ]
      },
      {
        "file": "/var/www/html/apps/announcementcenter/lib/BackgroundJob.php",
        "line": 212,
        "function": "callForSeenUsers",
        "class": "OC\\User\\Manager",
        "type": "->"
      },
      {
        "file": "/var/www/html/apps/announcementcenter/lib/BackgroundJob.php",
        "line": 151,
        "function": "createPublicityEveryone",
        "class": "OCA\\AnnouncementCenter\\BackgroundJob",
        "type": "->"
      },
      {
        "file": "/var/www/html/apps/announcementcenter/lib/BackgroundJob.php",
        "line": 114,
        "function": "createPublicity",
        "class": "OCA\\AnnouncementCenter\\BackgroundJob",
        "type": "->"
      },
      {
        "file": "/var/www/html/lib/public/BackgroundJob/Job.php",
        "line": 79,
        "function": "run",
        "class": "OCA\\AnnouncementCenter\\BackgroundJob",
        "type": "->"
      },
      {
        "file": "/var/www/html/lib/public/BackgroundJob/QueuedJob.php",
        "line": 47,
        "function": "execute",
        "class": "OCP\\BackgroundJob\\Job",
        "type": "->"
      },
      {
        "file": "/var/www/html/cron.php",
        "line": 151,
        "function": "execute",
        "class": "OCP\\BackgroundJob\\QueuedJob",
        "type": "->"
      }
    ],
    "File": "/var/www/html/lib/private/Log/ErrorHandler.php",
    "Line": 92,
    "CustomMessage": "--"
  }
}
{
  "reqId": "",
  "level": 3,
  "time": "",
  "remoteAddr": "",
  "user": "--",
  "app": "cron",
  "method": "",
  "url": "--",
  "message": "idn_to_ascii() expects parameter 1 to be string, null given",
  "userAgent": "--",
  "version": "23.0.9.1",
  "exception": {
    "Exception": "TypeError",
    "Message": "idn_to_ascii() expects parameter 1 to be string, null given",
    "Code": 0,
    "Trace": [
      {
        "file": "/var/www/html/lib/private/Mail/Message.php",
        "line": 87,
        "function": "idn_to_ascii"
      },
      {
        "file": "/var/www/html/lib/private/Mail/Message.php",
        "line": 148,
        "function": "convertAddresses",
        "class": "OC\\Mail\\Message",
        "type": "->"
      },
      {
        "file": "/var/www/html/apps/announcementcenter/lib/BackgroundJob.php",
        "line": 205,
        "function": "setTo",
        "class": "OC\\Mail\\Message",
        "type": "->"
      },
      {
        "file": "/var/www/html/lib/private/User/Manager.php",
        "line": 656,
        "function": "OCA\\AnnouncementCenter\\{closure}",
        "class": "OCA\\AnnouncementCenter\\BackgroundJob",
        "type": "->",
        "args": [
          "*** sensitive parameters replaced ***"
        ]
      },
      {
        "file": "/var/www/html/apps/announcementcenter/lib/BackgroundJob.php",
        "line": 212,
        "function": "callForSeenUsers",
        "class": "OC\\User\\Manager",
        "type": "->"
      },
      {
        "file": "/var/www/html/apps/announcementcenter/lib/BackgroundJob.php",
        "line": 151,
        "function": "createPublicityEveryone",
        "class": "OCA\\AnnouncementCenter\\BackgroundJob",
        "type": "->"
      },
      {
        "file": "/var/www/html/apps/announcementcenter/lib/BackgroundJob.php",
        "line": 114,
        "function": "createPublicity",
        "class": "OCA\\AnnouncementCenter\\BackgroundJob",
        "type": "->"
      },
      {
        "file": "/var/www/html/lib/public/BackgroundJob/Job.php",
        "line": 79,
        "function": "run",
        "class": "OCA\\AnnouncementCenter\\BackgroundJob",
        "type": "->"
      },
      {
        "file": "/var/www/html/lib/public/BackgroundJob/QueuedJob.php",
        "line": 47,
        "function": "execute",
        "class": "OCP\\BackgroundJob\\Job",
        "type": "->"
      },
      {
        "file": "/var/www/html/cron.php",
        "line": 151,
        "function": "execute",
        "class": "OCP\\BackgroundJob\\QueuedJob",
        "type": "->"
      }
    ],
    "File": "/var/www/html/lib/private/Mail/Message.php",
    "Line": 87,
    "CustomMessage": "--"
  }

@juliusknorr
Copy link
Member Author

Thinking about that maybe we should rather do that in setTo but that would introduce an API change since throwing on that function call is currently not expected.

Or if converting the addresses fails, we just return the passed array and let further error handling happen when actually sending the mail

https://github.com/nextcloud/server/blob/215aef3cbdc1963be1bb6bca5218ee0a4b7f1665/lib/private/Mail/Message.php#L75

@nickvergessen
Copy link
Member

/backport to stable24

@nickvergessen
Copy link
Member

OCA\AnnouncementCenter\Tests\BackgroundJobTest::testCreatePublicityEveryone with data set #0 (array(true, false, true), true, false, true)
Expectation failed for method name is "send" when invoked 3 time(s).
Method was expected to be called 3 times, actually called 0 times.

😆

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
Copy link
Member Author

Tests should pass now and a server fix was suggested in nextcloud/server#34775

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.

3 participants