search

plp050452 / simplesamlphp

Automatically exported from code.google.com/p/simplesamlphp
Other
0 stars 0 forks source link

Patch for /trunk/lib/SimpleSAML/Auth/LDAP.php #445

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago 
The escaping of the attribute was set to always return an array, which in turn 
causes the next if statement to always add (|.

Original issue reported on code.google.com by pan...@traileyes.com on 31 Oct 2011 at 8:48

Attachments:

GoogleCodeExporter commented 8 years ago 
Your patch breaks the function in case we are searching attributes. (Removing 
"FALSE" makes it always return a single string.)

But: Is the extra OR-"operator" a problem? As far as I can tell (|(asdf=asdf)) 
is still a valid LDAP query.

Original comment by olavmrk@gmail.com on 4 Nov 2011 at 2:22

GoogleCodeExporter commented 8 years ago 
Ok, I see. What about changing the if/else statements to array counts? Attached 
is a patch which gives you the idea. I couldn't download the Google Code diff 
to attach.

Original comment by pan...@traileyes.com on 4 Nov 2011 at 3:40

Attachments:

GoogleCodeExporter commented 8 years ago 
Oh, and No I don't think the extra | operator makes a bid difference. Just a 
technicality really. I just thought the if/else logic was un-necessary if it 
doesn't do anything.

Original comment by pan...@traileyes.com on 4 Nov 2011 at 7:31

GoogleCodeExporter commented 8 years ago 
I agree that the if-else-statements are unnecessary, but in that case I think 
the correct fix is to remove them. Unless the or-operator has a measurable 
overhead, I don't think trying to optimize the generated query makes sense.

Original comment by olavmrk@gmail.com on 7 Nov 2011 at 6:56

GoogleCodeExporter commented 8 years ago 
I would agree at that point, no need for extra code. Although I don't have 
stats on if it causes extra overhead or not, so I couldn't say for sure if it's 
a big deal. (IMO, not a big deal though.) Would you like a new patch? Else I'm 
sure it would be easy enough for you do do.

Original comment by pan...@traileyes.com on 11 Nov 2011 at 7:32

GoogleCodeExporter commented 8 years ago 
Attached is a patch to just remove the if/else logic, since the attribute will 
always be an array, returned from the escape method.

Original comment by pan...@traileyes.com on 14 Nov 2011 at 5:14

Attachments:

GoogleCodeExporter commented 8 years ago 
Thanks! I have applied the patch as r2982.

Original comment by olavmrk@gmail.com on 17 Nov 2011 at 9:56