Re: [Freeipa-devel] [PATCH] 0006 add context to exception on LdapEntry decode error
On 09.06.2016 14:31, Stanislav Laznicka wrote: On 06/09/2016 11:58 AM, Florence Blanc-Renaud wrote: On 06/08/2016 01:14 PM, Stanislav Laznicka wrote: On 06/08/2016 01:13 PM, Stanislav Laznicka wrote: On 06/07/2016 05:11 PM, Florence Blanc-Renaud wrote: On 06/07/2016 04:08 PM, Stanislav Laznicka wrote: On 06/06/2016 02:47 PM, Florence Blanc-Renaud wrote: Hi, please find attached the patch for Ticket 5434 add context to exception on LdapEntry decode error https://fedorahosted.org/freeipa/ticket/5434 Hello, Similarly to you patch 0003, could you please shorten the commit message? I appreciate that you added the way to reproduce the bug but I would rather see it in the comments of the ticket. I haven't tested the patch yet but I think that the try-except block should also wrap the self._conn.decode in the raw_dels loop: 310 for value in raw_dels: 311 value = self._conn.decode(value, name) 312 if value in nice_adds: 313 continue 314 nice.remove(value) Standa Hi Standa, thanks for your review. I will shorten the commit message as you suggested. Regarding the other decode() called for raw_dels, I was not sure in which case this part of the code was called. Do you happen to know if it possible for an invalid value to be in the raw_sync list but not in the raw list? Flo. I'm not sure if there's an easy way to do that. I would just put it there should it ever occur to have more information about what happened. Forgot to include devel-list, sorry. Hi, updated patch attached. Flo. Thank you for the updated patch. It seems to work as expected so ACK. Pushed to master: 53524fbbff51418a8f1194c8559c9dcfcc5bbb83 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0006 add context to exception on LdapEntry decode error
On 06/09/2016 11:58 AM, Florence Blanc-Renaud wrote: On 06/08/2016 01:14 PM, Stanislav Laznicka wrote: On 06/08/2016 01:13 PM, Stanislav Laznicka wrote: On 06/07/2016 05:11 PM, Florence Blanc-Renaud wrote: On 06/07/2016 04:08 PM, Stanislav Laznicka wrote: On 06/06/2016 02:47 PM, Florence Blanc-Renaud wrote: Hi, please find attached the patch for Ticket 5434 add context to exception on LdapEntry decode error https://fedorahosted.org/freeipa/ticket/5434 Hello, Similarly to you patch 0003, could you please shorten the commit message? I appreciate that you added the way to reproduce the bug but I would rather see it in the comments of the ticket. I haven't tested the patch yet but I think that the try-except block should also wrap the self._conn.decode in the raw_dels loop: 310 for value in raw_dels: 311 value = self._conn.decode(value, name) 312 if value in nice_adds: 313 continue 314 nice.remove(value) Standa Hi Standa, thanks for your review. I will shorten the commit message as you suggested. Regarding the other decode() called for raw_dels, I was not sure in which case this part of the code was called. Do you happen to know if it possible for an invalid value to be in the raw_sync list but not in the raw list? Flo. I'm not sure if there's an easy way to do that. I would just put it there should it ever occur to have more information about what happened. Forgot to include devel-list, sorry. Hi, updated patch attached. Flo. Thank you for the updated patch. It seems to work as expected so ACK. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0006 add context to exception on LdapEntry decode error
On 06/08/2016 01:14 PM, Stanislav Laznicka wrote: On 06/08/2016 01:13 PM, Stanislav Laznicka wrote: On 06/07/2016 05:11 PM, Florence Blanc-Renaud wrote: On 06/07/2016 04:08 PM, Stanislav Laznicka wrote: On 06/06/2016 02:47 PM, Florence Blanc-Renaud wrote: Hi, please find attached the patch for Ticket 5434 add context to exception on LdapEntry decode error https://fedorahosted.org/freeipa/ticket/5434 Hello, Similarly to you patch 0003, could you please shorten the commit message? I appreciate that you added the way to reproduce the bug but I would rather see it in the comments of the ticket. I haven't tested the patch yet but I think that the try-except block should also wrap the self._conn.decode in the raw_dels loop: 310 for value in raw_dels: 311 value = self._conn.decode(value, name) 312 if value in nice_adds: 313 continue 314 nice.remove(value) Standa Hi Standa, thanks for your review. I will shorten the commit message as you suggested. Regarding the other decode() called for raw_dels, I was not sure in which case this part of the code was called. Do you happen to know if it possible for an invalid value to be in the raw_sync list but not in the raw list? Flo. I'm not sure if there's an easy way to do that. I would just put it there should it ever occur to have more information about what happened. Forgot to include devel-list, sorry. Hi, updated patch attached. Flo. From 04d591a2fc0f0e0ae3139aa8f1eccafbe26905aa Mon Sep 17 00:00:00 2001 From: Florence Blanc-RenaudDate: Wed, 8 Jun 2016 18:09:15 +0200 Subject: [PATCH] add context to exception on LdapEntry decode error When reading the content of an invalid LDAP entry, the exception only displays the attribute name and value, but not the DN of the entry. Because of this, it is difficult to identify the root cause of the problem. The fix raises a ValueError exception which also contains the entry DN. https://fedorahosted.org/freeipa/ticket/5434 --- ipapython/ipaldap.py | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py index 9fb7fd3f5a49dec0fd855c05f0e64004593e1306..410ddae2c484f060abf2bbf3e897549a26b0ebc9 100644 --- a/ipapython/ipaldap.py +++ b/ipapython/ipaldap.py @@ -308,7 +308,11 @@ class LDAPEntry(collections.MutableMapping): raw.remove(value) for value in raw_dels: -value = self._conn.decode(value, name) +try: +value = self._conn.decode(value, name) +except ValueError as e: +raise ValueError("{error} in LDAP entry '{dn}'".format( +error=e, dn=self._dn)) if value in nice_adds: continue nice.remove(value) @@ -320,7 +324,11 @@ class LDAPEntry(collections.MutableMapping): raw.append(value) for value in raw_adds: -value = self._conn.decode(value, name) +try: +value = self._conn.decode(value, name) +except ValueError as e: +raise ValueError("{error} in LDAP entry '{dn}'".format( +error=e, dn=self._dn)) if value in nice_dels: continue nice.append(value) -- 2.5.5 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0006 add context to exception on LdapEntry decode error
On 06/08/2016 01:13 PM, Stanislav Laznicka wrote: On 06/07/2016 05:11 PM, Florence Blanc-Renaud wrote: On 06/07/2016 04:08 PM, Stanislav Laznicka wrote: On 06/06/2016 02:47 PM, Florence Blanc-Renaud wrote: Hi, please find attached the patch for Ticket 5434 add context to exception on LdapEntry decode error https://fedorahosted.org/freeipa/ticket/5434 Hello, Similarly to you patch 0003, could you please shorten the commit message? I appreciate that you added the way to reproduce the bug but I would rather see it in the comments of the ticket. I haven't tested the patch yet but I think that the try-except block should also wrap the self._conn.decode in the raw_dels loop: 310 for value in raw_dels: 311 value = self._conn.decode(value, name) 312 if value in nice_adds: 313 continue 314 nice.remove(value) Standa Hi Standa, thanks for your review. I will shorten the commit message as you suggested. Regarding the other decode() called for raw_dels, I was not sure in which case this part of the code was called. Do you happen to know if it possible for an invalid value to be in the raw_sync list but not in the raw list? Flo. I'm not sure if there's an easy way to do that. I would just put it there should it ever occur to have more information about what happened. Forgot to include devel-list, sorry. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
[Freeipa-devel] [PATCH] 0006 add context to exception on LdapEntry decode error
Hi, please find attached the patch for Ticket 5434 add context to exception on LdapEntry decode error https://fedorahosted.org/freeipa/ticket/5434 From 8094fca2e0a11c1c108959da3a8f05c3d9c62bb7 Mon Sep 17 00:00:00 2001 From: Florence Blanc-RenaudDate: Fri, 3 Jun 2016 14:56:35 +0200 Subject: [PATCH] add context to exception on LdapEntry decode error When reading the content of an invalid LDAP entry, the exception only displays the attribute name and value, but not the DN of the entry. Because of this, it is difficult to identify the root cause of the problem. The fix raises a ValueError exception which also contains the entry DN. Note that this does not change the overall behavior: the web browser will still display An error has occurred (IPA Error 903: InternalError) Please try the following options: Refresh the page. Return to the main page and retry the operation Reload the browser. If the problem persists please contact the system administrator. The exception is printed in httpd/error_log as ValueError: unable to convert the attribute u'memberofindirect' value 'Y249ZWV5ZV9ob3N0LGNuPWhvc3Rncm91cHMsY249YWNjb3VudHMsZGM9cHJvZCxkYLg' to type in LDAP entry 'fqdn=vm-058-124.abc.idm.lab.eng.brq.redhat.com,cn=computers,cn=accounts,dc=dom-058-232,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com' The issue can be reproduced by disabling the schema checking and manually modifying a host entry with a value that is not base64-decodable. Example ldif: dn: cn=config changetype: modify replace: nsslapd-syntaxcheck nsslapd-syntaxcheck: off dn: fqdn=,cn=computers,cn=accounts, changetype: modify add: memberof memberof: Y249ZWV5ZV9ob3N0LGNuPWhvc3Rncm91cHMsY249YWNjb3VudHMsZGM9cHJvZCxkYLg https://fedorahosted.org/freeipa/ticket/5434 --- ipapython/ipaldap.py | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py index 9fb7fd3f5a49dec0fd855c05f0e64004593e1306..6bade5bbc6f87ba9dd3d99b6f6befbfa2ee086c5 100644 --- a/ipapython/ipaldap.py +++ b/ipapython/ipaldap.py @@ -320,7 +320,11 @@ class LDAPEntry(collections.MutableMapping): raw.append(value) for value in raw_adds: -value = self._conn.decode(value, name) +try: +value = self._conn.decode(value, name) +except ValueError as e: +raise ValueError("{error} in LDAP entry '{dn}'".format( +error=e, dn=self._dn)) if value in nice_dels: continue nice.append(value) -- 2.5.5 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code