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(notification): Prevent null in parameters #4908

Merged

Conversation

nickvergessen
Copy link
Member

  • Resolves: #
  • Target version: main

Summary

In our instance we have a notification record like this:

|         1236905 | deck               | m…a | 1688536306 | board                 | 371        | board-shared        | ["A…s",null]                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                         |                     | []                      |                                                                                 | []      |                                                                                     |

The null here is then breaking when trying to send the notification email:

{
  "reqId": "mdWuqhrBH4ueDGB9oH5R",
  "level": 3,
  "time": "2023-07-14T05:40:02+00:00",
  "remoteAddr": "",
  "user": "--",
  "app": "notifications",
  "method": "",
  "url": "--",
  "message": "An error occurred while preparing a notification (deck|board-shared|board|371) for sending",
  "userAgent": "--",
  "version": "27.0.1.0",
  "exception": {
    "Exception": "TypeError",
    "Message": "htmlspecialchars(): Argument #1 ($string) must be of type string, null given",
    "Code": 0,
    "Trace": [
      {
        "file": "…/apps/notifications/lib/MailNotifications.php",
        "line": 386,
        "function": "htmlspecialchars",
        "args": [
          null
        ]
      },
      {
        "file": "…/apps/notifications/lib/MailNotifications.php",
        "line": 347,
        "function": "replaceRichParameters",
        "class": "OCA\\Notifications\\MailNotifications",
        "type": "->",
        "args": [
          [
            [
              "deck-board",
              "371",
              "A…s",
              "https://…/index.php/apps/deck/#/board/371"
            ],
            [
              "user",
              null,
              null
            ]
          ],
          "{user} has shared {deck-board} with you."
        ]
      },

I don't know where the bug actually was created, maybe by ownership transfer or a share via CLI or something. But by defaulting to empty string the email at least sends

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

Signed-off-by: Joas Schilling <coding@schilljs.com>
@cypress
Copy link

cypress bot commented Jul 14, 2023

Passing run #1116 ↗︎

0 15 0 0 Flakiness 0

Details:

Merge 4b1e670 into 85e09ac...
Project: Deck Commit: 3e959a2056 ℹ️
Status: Passed Duration: 01:16 💡
Started: Jul 14, 2023 6:02 AM Ended: Jul 14, 2023 6:03 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@juliushaertl juliushaertl merged commit 90d051f into main Jul 14, 2023
25 checks passed
@juliushaertl juliushaertl deleted the bugfix/noid/prevent-null-in-notification-parameter branch July 14, 2023 06:25
@juliushaertl
Copy link
Member

/backport to stable27

@juliushaertl
Copy link
Member

/backport to stable26

@juliushaertl
Copy link
Member

/backport to stable25

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants