Bug 20044 - Exceptions in Exceptionhandler werden nicht abgefangen (KeyError: 'sambaSID')
Exceptions in Exceptionhandler werden nicht abgefangen (KeyError: 'sambaSID')
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UDM (Generic)
UCS 2.4
Other Linux
: P5 normal (vote)
: UCS 4.0-0-errata
Assigned To: Florian Best
Dirk Wiesenthal
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-20 10:26 CEST by Sönke Schwardt-Krummrich
Modified: 2015-09-28 09:51 CEST (History)
6 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Ticket number:
Bug group (optional): Error handling, External feedback
Max CVSS v3 score:
best: Patch_Available+


Attachments
patch (2.20 KB, patch)
2015-01-12 18:35 CET, Florian Best
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sönke Schwardt-Krummrich univentionstaff 2010-09-20 10:26:18 CEST
Wenn ein Objekt angelegt wird und eine post_create-Funktion existiert, wird nachfolgender Code ausgeführt.
Dieser fängt jedoch keine Exceptions in self.cancel() oder self.remove() sinnvoll ab. Das führt dann z.B. zu dem Traceback unten.


---[univention/admin/handlers/__init__.py:649]---
 if hasattr(self,'_ldap_post_create'):
         # if anything goes wrong we need to remove the already created object, otherwise we run into 'already exists' errors
         try:    
                 self._ldap_post_create()
         except Exception, e:
                 # ensure that there is no lock left
                 self.cancel()
                 self.remove()
                 raise e
---[cut]---

Traceback (most recent call last):
  File "/usr/share/ucs-school-import/scripts/import_user", line 1280, in create_user
    exists, dn = create_object(object)
  File "/usr/share/ucs-school-import/scripts/import_user", line 793, in create_object
    dn=o.create()
  File "/usr/lib/python2.4/site-packages/univention/admin/handlers/__init__.py", line 307, in create
    return self._create()
  File "/usr/lib/python2.4/site-packages/univention/admin/handlers/__init__.py", line 649, in _create
    self.remove()
  File "/usr/lib/python2.4/site-packages/univention/admin/handlers/__init__.py", line 431, in remove
    return self._remove(remove_childs)
  File "/usr/lib/python2.4/site-packages/univention/admin/handlers/__init__.py", line 853, in _remove
    self._ldap_pre_remove()
  File "/usr/lib/python2.4/site-packages/univention/admin/handlers/users/user.py", line 2772, in _ldap_pre_remove
    self.sid=self.oldattr['sambaSID'][0]
KeyError: 'sambaSID'
Comment 1 Tim Petersen univentionstaff 2012-05-22 08:15:03 CEST
Erneut an Ticket #2012051621002077 gemeldet. UCS 2.4-3
Comment 2 Tim Petersen univentionstaff 2012-05-22 08:21:39 CEST
An oben erwähntem Ticket wurde der Benutzer allerdings entfernt, nicht hinzugefügt.

Traceback (most recent call last): 
  File "/usr/share/univention-webui/modules/requests.py", line 272, in
run_request 
    self.dialog.apply() 
  File "./unidialog.py", line 615, in apply 
    self.mod.apply() 
  File "/usr/share/univention-directory-manager/uniconf/modwizard.py",
line 1703, in apply 
    object.remove(remove_childs) 
  File
"/usr/lib/python2.4/site-packages/univention/admin/handlers/__init__.py"
, line 430, in remove 
    return self._remove(remove_childs) 
  File
"/usr/lib/python2.4/site-packages/univention/admin/handlers/__init__.py"
, line 852, in _remove 
    self._ldap_pre_remove() 
  File
"/usr/lib/python2.4/site-packages/univention/admin/handlers/users/user.p
y", line 2785, in _ldap_pre_remove 
    self.sid=self.oldattr['sambaSID'][0] 
KeyError: 'sambaSID'
Comment 3 Sönke Schwardt-Krummrich univentionstaff 2012-05-22 11:09:26 CEST
(In reply to comment #2)
> An oben erwähntem Ticket wurde der Benutzer allerdings entfernt, nicht
> hinzugefügt.

Bitte mal prüfen:
Der Traceback kommt auch, wenn das zu entfernende Benutzerobjekt im LDAP gar nicht existiert.
Comment 4 Stefan Gohmann univentionstaff 2013-04-03 06:59:34 CEST
Tritt das mit UCS 3 noch auf?
Comment 5 Florian Best univentionstaff 2014-09-01 10:47:45 CEST
(In reply to Stefan Gohmann from comment #4)
> Tritt das mit UCS 3 noch auf?
yes
Comment 6 Sönke Schwardt-Krummrich univentionstaff 2014-09-05 10:04:22 CEST
Der Traceback ist an Bug 35759 erneut aufgetreten. Echtes Problem ist hier, dass im Exception-Handler beim Aufräumen wieder eine Exception auftritt. Und das tut sie zuverlässig, wenn das ldap_add nicht funktioniert hat. Damit wird die eigentliche Exception überlagert und man hat keine Ahnung, wo und warum der ursprüngliche Fehler aufgetreten ist.

Die remove() Funktion muss noch vor dem Aufruf von pre_remove() prüfen, ob das Objekt überhaupt im LDAP existiert. Ansonsten sollte sie einfach nichts machen.

Außerdem sollte der Exception-Handler kein "raise e" sondern ein einfaches "raise" ausführen, um die gefangene Exception nach dem Aufräumen nach oben durchzuwerfen.
Comment 7 Philipp Hahn univentionstaff 2014-09-05 10:15:38 CEST
(In reply to Florian Best from comment #5)
> (In reply to Stefan Gohmann from comment #4)
> > Tritt das mit UCS 3 noch auf?
> yes

Also seen with UCS-4
Comment 8 Florian Best univentionstaff 2014-12-15 10:12:04 CET
Reported again.
Comment 9 Florian Best univentionstaff 2015-01-09 11:39:04 CET
Reported again, 3.2-4 errata247 (Borgfeld)
Comment 10 Florian Best univentionstaff 2015-01-12 18:35:15 CET
Created attachment 6593 [details]
patch

patch:
* raise noObject again when the object does not exists
* log exceptions which occur during reverting the changes
* preserve traceback of original exception

@Sönke: what do you think about the patch?
Comment 11 Florian Best univentionstaff 2015-01-22 17:01:03 CET
Enhanced patch has been applied in svn r57502.

YAML: 2015-01-22-univention-directory-manager-modules.yaml
Package: univention-directory-manager-modules
Version: 10.0.29-19.1283.201501221644
Branch: ucs_4.0-0
Scope: errata4.0-0
no cross dependencies.
Comment 12 Dirk Wiesenthal univentionstaff 2015-01-27 14:06:26 CET
This does not work.

Just "raise" seems correct, but it will actually reraise the cancel/remove error (if any).

Furthermore, where is this logged? I have totally messed up _ldap_post_create() and cancel(). But I only have

Post-Create operation failed: Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.7/univention/admin/handlers/__init__.py", line 790, in _create
    self._ldap_post_create()
  File "/usr/lib/pymodules/python2.7/univention/admin/handlers/users/user.py", line 1878, in _ldap_post_create
    if 'samba' in self.option:
AttributeError: 'object' object has no attribute 'option'

in /var/log/univention/directory-manager-cmd.log.

I would also doubt that this is the whole stack.

UDM itself now reports an error in remove(). Is remove safe to be called when cancel() raised? So far, cancel() is defined to remove allocated objects. If this not work somehow, should the object be removed?
Comment 13 Florian Best univentionstaff 2015-01-27 14:51:44 CET
Ok, reraising is now fixed (package currently builds).
Also the loglevel for the subexceptions have been lowered to be always logged.
For the rest: make sure you do 'pkill -f univention-cli-server' after changing files and make sure you don't use objects which have a lock when testing.
Comment 14 Dirk Wiesenthal univentionstaff 2015-01-27 15:32:50 CET
OK, works as expected.
Comment 15 Janek Walkenhorst univentionstaff 2015-01-29 11:44:49 CET
<http://errata.univention.de/ucs/4.0/65.html>
Comment 16 Florian Best univentionstaff 2015-09-14 17:52:18 CEST
Reported again, 3.2-6 errata344 (Borgfeld)
Comment 17 Florian Best univentionstaff 2015-09-28 09:51:47 CEST
(In reply to Florian Best from comment #16)
> Reported again, 3.2-6 errata344 (Borgfeld)
same UUID again.