Moodle

Warning shown when debugging is on and user doesn't exist in LDAP Context

Details

Description

We have recently turned on full debugging in our TEST environment and found the warning message prevents redirection:

Warning: ldap_search(): Search: No such object in /var/www/moodle/lib/ldaplib.php on line 241

Warning: ldap_first_entry() expects parameter 2 to be resource, boolean given in /var/www/moodle/lib/ldaplib.php on line 248

Issue Links

has a non-specific relationship to

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-42723 Warning shown when debugging is on and user doesn't exist in LDAP Context (follow up to MDL-41304)

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Michael de Raadt added a comment -

Thanks for reporting that and providing patches. Feel free to put that up for peer review.

You might want to add a bit more detail to the testing instructions. Most of the devs around here have limited LDAP experience.

Show
Michael de Raadt added a comment - Thanks for reporting that and providing patches. Feel free to put that up for peer review. You might want to add a bit more detail to the testing instructions. Most of the devs around here have limited LDAP experience.
Hide
Permalink
Mark Nelson added a comment -

Hi Tim,

We tend to avoid using @ to suppress warnings with any newly introduced code, however in this case I see no other alternative. The logic seems fine to me.

Thanks for your contribution.

Regards,

Mark

Show
Mark Nelson added a comment - Hi Tim, We tend to avoid using @ to suppress warnings with any newly introduced code, however in this case I see no other alternative. The logic seems fine to me. Thanks for your contribution. Regards, Mark
Hide
Permalink
Tim Lock added a comment -

Hi Mark,

Thanks and I agree that silencing errors with an @ should be avoided if possible.

Show
Tim Lock added a comment - Hi Mark, Thanks and I agree that silencing errors with an @ should be avoided if possible.
Hide
Permalink
Dan Poltawski added a comment -

Thanks Tim - i've integrated it to master, 25 and 24

Show
Dan Poltawski added a comment - Thanks Tim - i've integrated it to master, 25 and 24
Hide
Permalink
Rajesh Taneja added a comment -

Thanks for fixing this Tim,

Passing test...

Show
Rajesh Taneja added a comment - Thanks for fixing this Tim, Passing test...
Hide
Permalink
Dan Poltawski added a comment -

Congratulations! This change has been integrated upstream and is now available from our git and download mirrors. To celebrate, here is a joke:

A SQL query goes into a bar, walks up to two tables and asks, "Can I join you?"

Show
Dan Poltawski added a comment - Congratulations! This change has been integrated upstream and is now available from our git and download mirrors. To celebrate, here is a joke: A SQL query goes into a bar, walks up to two tables and asks, "Can I join you?"
Hide
Permalink
Iñaki Arenaza added a comment -

I hate to re-open this, but the fix is incomplete. If you configure the LDAP auth plugin not to search subcontexts, you get the same warnings (under the same circumstances). So the check for $ldap_result and the break sentence should go after the "if then else" block, before the call to ldap_first_entry(). And we would also need to silence the call to ldap_list() for the same reasons.

I'll prepare patches in a few hours and will put the links to github here.

Show
Iñaki Arenaza added a comment - I hate to re-open this, but the fix is incomplete. If you configure the LDAP auth plugin not to search subcontexts, you get the same warnings (under the same circumstances). So the check for $ldap_result and the break sentence should go after the "if then else" block, before the call to ldap_first_entry(). And we would also need to silence the call to ldap_list() for the same reasons. I'll prepare patches in a few hours and will put the links to github here.
Hide
Permalink
Dan Poltawski added a comment -

Hi Iñaki,

Can we create a new issue for this, as reopening the issue doesn't really work with our workflow.

thanks

Show
Dan Poltawski added a comment - Hi Iñaki, Can we create a new issue for this, as reopening the issue doesn't really work with our workflow. thanks
Hide
Permalink
Iñaki Arenaza added a comment -

Sure, Dan.

Show
Iñaki Arenaza added a comment - Sure, Dan.
Hide
Permalink
Dan Poltawski added a comment -

Thanks, sending this back to closed. Please link it to here.

Show
Dan Poltawski added a comment - Thanks, sending this back to closed. Please link it to here.

People

Vote (0)
Watch (5)

Dates

  • Created:
    Updated:
    Resolved: