Skip to content

fix(user_ldap): Harmonize parameter obfuscation and serialization accross logging methods - #53250

Merged
come-nc merged 1 commit into
masterfrom
fix/harmonize-ldap-function-logging
Jun 5, 2025
Merged

fix(user_ldap): Harmonize parameter obfuscation and serialization accross logging methods#53250
come-nc merged 1 commit into
masterfrom
fix/harmonize-ldap-function-logging

Conversation

@come-nc

Copy link
Copy Markdown
Contributor

Summary

Debug log, profiler and ldap debug log had a different logic for
sanitizing of parameters, aligning them.

Checklist

…ross logging methods
Debug log, profiler and ldap debug log had a different logic for
sanitizing of parameters, aligning them.
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nccome-nc added this to the Nextcloud 32 milestone Jun 2, 2025
@come-nccome-nc self-assigned this Jun 2, 2025
@come-nccome-nc added the 3. to review Waiting for reviews label Jun 2, 2025
@come-nc
come-nc requested a review from a team as a code ownerJune 2, 2025 09:49
@come-nc
come-nc requested review from ArtificialOwl, icewind1991 and sorbaugh and removed request for a teamJune 2, 2025 09:49
@come-nc

Copy link
Copy Markdown
ContributorAuthor

/backport to stable31

@come-nc

Copy link
Copy Markdown
ContributorAuthor

/backport to stable30

@come-nc

Copy link
Copy Markdown
ContributorAuthor

/backport to stable29

@come-nc
come-nc requested review from blizzz and susnuxJune 3, 2025 13:01

@artongeartonge left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fine beside one question

'func' => $functionName,
'args' => json_encode($args),
]);
if ($this->config->getSystemValue('loglevel') === ILogger::DEBUG) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why is this needed if we then call logger->debug()?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

sanitizeFunctionParameters felt a bit expensive to run for nothing, so I wanted to only use the computing time if the log is actually gonna happen.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Actually the comment says just that on the line below: /* Only running this if debug loglevel is on, to avoid processing parameters on production */

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@come-nc@artonge@miaulalala@skjnldsv