Skip to content

Trim the subject before encrypting the subject#383

Merged
nickvergessen merged 1 commit into
masterfrom
bugfix/346/trim-subject-before-encrypting
Jul 16, 2019
Merged

Trim the subject before encrypting the subject#383
nickvergessen merged 1 commit into
masterfrom
bugfix/346/trim-subject-before-encrypting

Conversation

@nickvergessen

Copy link
Copy Markdown
Member

Fix #346

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen

Copy link
Copy Markdown
Member Author

/backport to stable16

@nickvergessen

Copy link
Copy Markdown
Member Author

/backport to stable15

@nickvergessen

Copy link
Copy Markdown
Member Author

/backport to stable14

@MorrisJobke MorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks good 👍

Comment thread lib/Push.php
$subject = $notification->getParsedSubject();
$dataLength = 245 - strlen(json_encode($data));
if (strlen($subject) > $dataLength) {
$data['subject'] = substr($subject, 0, $dataLength) . '…';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should the substr not be 1 shorter than the dataLength?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I started with 245, instead of 255 so we have a little buffer there

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah ok perfect. I wasn't sure if that was maybe for other metadata somewhere.

@nickvergessen nickvergessen merged commit a5de371 into master Jul 16, 2019
@delete-merged-branch delete-merged-branch Bot deleted the bugfix/346/trim-subject-before-encrypting branch July 16, 2019 14:40
@backportbot-nextcloud

Copy link
Copy Markdown

backport to stable16 in #384

@backportbot-nextcloud

Copy link
Copy Markdown

backport to stable15 in #385

@backportbot-nextcloud

Copy link
Copy Markdown

backport to stable14 in #386

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.

NC16.0.1 - Notification (app 2.4.1) broken

3 participants