Skip to content

Conversation

@blizzz
Copy link
Member

@blizzz blizzz commented Nov 5, 2018

When a search request containing a limit is sent against an LDAP server, most will sent the message as in $subject along with the results. The LDAP PHP module triggers this as an PHP error, thus ending up through our error handler in the Nextcloud log.

While not being an error in itself, often admins perceive this as confusing, ending up in several requests not only in this issue tracker. Hence, it would be nice to filter out this message, like this PR does. Fixes #2947

An alternative approach would be to open up our internal error handler from core to app specific proceedings.

How to test:

  1. It works with a new as well as existing LDAP config
  2. Go to the LDAP wizard
  3. Watch the Nextcloud log
  4. Open the "Login attributes" tab

Before the patch, the mentioned error message would be written. Now it is not supposed to happen anymore.

… at"

LDAP servers respond with that even if a limit was passed with the
request. Having this statement logged causes a lot of confusion.

Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz blizzz force-pushed the fix/2947/lapse-sizelimit-error branch from 931946f to deec5a7 Compare November 7, 2018 12:16
@MorrisJobke MorrisJobke mentioned this pull request Nov 8, 2018
24 tasks
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

@MorrisJobke
Copy link
Member

@rullzer Mind to review this one?

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

🐘

@MorrisJobke MorrisJobke merged commit 159d759 into master Nov 15, 2018
@MorrisJobke MorrisJobke deleted the fix/2947/lapse-sizelimit-error branch November 15, 2018 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants