Re: [Zope-dev] Re: PermissionGeddon
Hi all, Em Dom, 2005-11-27 às 21:26 +0100, Florent Guillaume escreveu: Dieter Maurer wrote: The first change is in the manage_pasteObjects method of CopyContainer. There are some _setObject and _delObject calls which grew a new suppress_events parameter. [...] Several Folder like classes are likely to overwrite _setObject and _delObject. Maybe, the code that calls these methods with an additional parameter should be prepared to meet implementations that do not support the extra parameter. Maybe. But on the other hand I'd rather not have object manager code slowed down and uglified to suit the negligibly small number of classes that are in this case, and that can be trivialy upgraded in a forward-compatible manner. Not gathering crust is a nice an laudable goal, but so is keep backward compatibility. I humbly suggest that the workaround code on ObjectManager be created with a deprecation warning whenever it's triggered, declaring that the backward compatibility will go away in, say, version 2.11, when it won't be uglified and slowed down anymore. You are, in essence, changing the API. IMHO this should take the same deprecation treatment as everything else. Cheers, Leo. ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: PermissionGeddon
Dieter Maurer wrote: Hanno Schlichting wrote at 2005-11-26 09:28 +0100: ... I hope to have tracked the ~200 failing tests down to two of your changes in OFS.CopySupport. The first change is in the manage_pasteObjects method of CopyContainer. There are some _setObject and _delObject calls which grew a new suppress_events parameter. This breaks the reference implementation of Archetypes because it uses something based on BTreeFolder2 to store references and BTreeFolder2 overwrites both _setObject and _delObject with its own versions. Several Folder like classes are likely to overwrite _setObject and _delObject. Maybe, the code that calls these methods with an additional parameter should be prepared to meet implementations that do not support the extra parameter. Ok, due to popular demand I'll make such a change. I'm a bit peeved though at the lack of willingness from the few people that have reimplemented their version of _setObject/_delObject (which could be considered private APIs, seeing that they're prefixed with an underscore) to just modify their code for forward compatibility and be done with it, but instead have us embark in a year-long deprecation strategy. This is supposed to be open source, can't we be reactive to change in such situation? Are folks really going to ship their framework code with _setObject unmodified from the current version when they ship it for Five 1.2 or Zope 2.9? Florent -- Florent Guillaume, Nuxeo (Paris, France) CTO, Director of RD +33 1 40 33 71 59 http://nuxeo.com [EMAIL PROTECTED] ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: PermissionGeddon
On 26 Nov 2005, at 09:28, Hanno Schlichting wrote:
The second change is actually related to your permission work.
First of all I have to thank you for your great work :) But I have
found one nasty thing.
CopySupport had the following security declaration:
__ac_permissions__=(('Copy or Move', (), ('Anonymous', 'Manager',)),)
...
Globals.default__class_init__(CopySource)
which changed into:
security = ClassSecurityInfo()
security.setPermissionDefault(copy_or_move, ('Anonymous', 'Manager'))
...
InitializeClass(CopySource)
Now the InitializeClass call is actually an alias for the former
Globals call, so no change here. But as you wrote yourself, you had
some trouble with the mysterious __ac_permissions format.
Looking at the actual code in App.class_init in the last paragraph
I'm quite sure that the former code did effectivly nothing so far.
The actual setattr call is inside a 'for mname in mnames:' loop
where mnames is the second element of each security tuple - in this
special case the mysterious () which results in not going through
the 'for mname in mnames:' loop at all.
Ok I just fixed SecurityInfo, could you update AccessControl/ and
recheck please?
Florent
--
Florent Guillaume, Nuxeo (Paris, France) Director of RD
+33 1 40 33 71 59 http://nuxeo.com [EMAIL PROTECTED]
___
Zope-Dev maillist - Zope-Dev@zope.org
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Re: PermissionGeddon
Hi Florent Em Ter, 2005-11-29 às 15:32 +0100, Florent Guillaume escreveu: [...] I'm a bit peeved though at the lack of willingness from the few people that have reimplemented their version of _setObject/_delObject (which could be considered private APIs, seeing that they're prefixed with an underscore) to just modify their code for forward compatibility and be done with it, but instead have us embark in a year-long deprecation strategy. This is supposed to be open source, can't we be reactive to change in such situation? Are folks really going to ship their framework code with _setObject unmodified from the current version when they ship it for Five 1.2 or Zope 2.9? They probably will change it, people don't like their code to generate deprecation warnings. But the greatest beneficiaries of the deprecation strategy are not the framework builders, but the users. Suppose a Zope change breaks, say, Plone (to pick two arbitrary examples :-). This means, that in order to upgrade to the next Zope version, I need to upgrade Plone first. If Plone, on the other hand, depends on Zope features that are only available in the newer Zope version, I'm forced to upgrade both layers of my running site simultaneously, making it much more expensive to calculate the migration overhead and procedures. I don't want to start a discussion about whose responsability is to keep compatibility with what software, but I, for one, prefer to upgrade the lower layers of my solutions before the upper layers if possible: Python before Zope, Zope before Plone, Linux kernel before glibc. This is not always possible, and there are loads of counter-examples, but if we can avoid forcing the poor user to upgrade more than one piece of software at a time, I think this is something we should try to achieve. Cheers, Leo ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: PermissionGeddon
Dieter Maurer wrote: Hanno Schlichting wrote at 2005-11-26 09:28 +0100: ... I hope to have tracked the ~200 failing tests down to two of your changes in OFS.CopySupport. The first change is in the manage_pasteObjects method of CopyContainer. There are some _setObject and _delObject calls which grew a new suppress_events parameter. This breaks the reference implementation of Archetypes because it uses something based on BTreeFolder2 to store references and BTreeFolder2 overwrites both _setObject and _delObject with its own versions. Several Folder like classes are likely to overwrite _setObject and _delObject. Maybe, the code that calls these methods with an additional parameter should be prepared to meet implementations that do not support the extra parameter. I checked in a backward compatibility check for this too. Florent -- Florent Guillaume, Nuxeo (Paris, France) CTO, Director of RD +33 1 40 33 71 59 http://nuxeo.com [EMAIL PROTECTED] ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: PermissionGeddon
Florent Guillaume wrote: Ok I just fixed SecurityInfo, could you update AccessControl/ and recheck please? Florent Hi Florent. All our unit tests pass again. I'm really looking forward for having a new Zope .0 release which might be compatible with an existing Plone release ;) Great work, thanks so much! Hanno ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Re: PermissionGeddon
Hanno Schlichting wrote at 2005-11-26 09:28 +0100: ... I hope to have tracked the ~200 failing tests down to two of your changes in OFS.CopySupport. The first change is in the manage_pasteObjects method of CopyContainer. There are some _setObject and _delObject calls which grew a new suppress_events parameter. This breaks the reference implementation of Archetypes because it uses something based on BTreeFolder2 to store references and BTreeFolder2 overwrites both _setObject and _delObject with its own versions. Several Folder like classes are likely to overwrite _setObject and _delObject. Maybe, the code that calls these methods with an additional parameter should be prepared to meet implementations that do not support the extra parameter. -- Dieter ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: PermissionGeddon
Dieter Maurer wrote: The first change is in the manage_pasteObjects method of CopyContainer. There are some _setObject and _delObject calls which grew a new suppress_events parameter. This breaks the reference implementation of Archetypes because it uses something based on BTreeFolder2 to store references and BTreeFolder2 overwrites both _setObject and _delObject with its own versions. Several Folder like classes are likely to overwrite _setObject and _delObject. Maybe, the code that calls these methods with an additional parameter should be prepared to meet implementations that do not support the extra parameter. Maybe. But on the other hand I'd rather not have object manager code slowed down and uglified to suit the negligibly small number of classes that are in this case, and that can be trivialy upgraded in a forward-compatible manner. Florent -- Florent Guillaume, Nuxeo (Paris, France) Director of RD +33 1 40 33 71 59 http://nuxeo.com [EMAIL PROTECTED] ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: PermissionGeddon
Florent Guillaume wrote:
I've done a big checkin to switch practically everything to the new-
style (actually they're 5 years old) security declarations.
I'd appreciate if another set of eyes could double-check everything;
while I've taken a number of steps to ensure I didn't make mistakes you
never know...
http://mail.zope.org/pipermail/zope-checkins/2005-November/030103.html
Thanks,
Florent
Hi Florent.
I have spent some time to run all unit tests of Plone (2.1.1) and all
it's core products on Zope 2.9 and look into the failing tests.
I hope to have tracked the ~200 failing tests down to two of your
changes in OFS.CopySupport.
The first change is in the manage_pasteObjects method of CopyContainer.
There are some _setObject and _delObject calls which grew a new
suppress_events parameter. This breaks the reference implementation of
Archetypes because it uses something based on BTreeFolder2 to store
references and BTreeFolder2 overwrites both _setObject and _delObject
with its own versions.
As I'm quite new to Zope internals I suppose fixing BTreeFolder2 is the
right thing here (adding the new parameter on both of its methods). But
this change might effect some more products which have overwritten these
methods as CopySupport is a base class of OFS.Folder and so probably of
every folderish type out there.
The second change is actually related to your permission work. First of
all I have to thank you for your great work :) But I have found one
nasty thing.
CopySupport had the following security declaration:
__ac_permissions__=(('Copy or Move', (), ('Anonymous', 'Manager',)),)
...
Globals.default__class_init__(CopySource)
which changed into:
security = ClassSecurityInfo()
security.setPermissionDefault(copy_or_move, ('Anonymous', 'Manager'))
...
InitializeClass(CopySource)
Now the InitializeClass call is actually an alias for the former Globals
call, so no change here. But as you wrote yourself, you had some trouble
with the mysterious __ac_permissions format.
Looking at the actual code in App.class_init in the last paragraph I'm
quite sure that the former code did effectivly nothing so far. The
actual setattr call is inside a 'for mname in mnames:' loop where mnames
is the second element of each security tuple - in this special case the
mysterious () which results in not going through the 'for mname in
mnames:' loop at all.
In the Plone unit tests this results in about 190 failing tests as
inserting an object into a folder isn't allowed anymore for normal
members because this triggers some code in a Referenceable class which
is based on CopySupport directly, which required no extra permission so
far but now is protected through the 'Copy or Move' permission.
While I guess that protecting the class with this permission has been
the intent it was not in effect so far. So this should be either
reconsidered or at least noted in the changelog. Changing this in
Archetypes shouldn't be a problem (though I'm very new to Zope's
permissions system).
I hope this is the kind of feedback you wanted to get other than GREAT
WORK! ;)
Hanno
---
IRC: hannosch
___
Zope-Dev maillist - Zope-Dev@zope.org
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Re: PermissionGeddon
On 26 Nov 2005, at 09:28, Hanno Schlichting wrote:
Florent Guillaume wrote:
I've done a big checkin to switch practically everything to the
new- style (actually they're 5 years old) security declarations.
I'd appreciate if another set of eyes could double-check
everything; while I've taken a number of steps to ensure I didn't
make mistakes you never know...
http://mail.zope.org/pipermail/zope-checkins/2005-November/
030103.html
Thanks,
Florent
Hi Florent.
I have spent some time to run all unit tests of Plone (2.1.1) and
all it's core products on Zope 2.9 and look into the failing tests.
Thanks a lot!
I hope to have tracked the ~200 failing tests down to two of your
changes in OFS.CopySupport.
The first change is in the manage_pasteObjects method of
CopyContainer. There are some _setObject and _delObject calls which
grew a new suppress_events parameter. This breaks the reference
implementation of Archetypes because it uses something based on
BTreeFolder2 to store references and BTreeFolder2 overwrites both
_setObject and _delObject with its own versions.
The BTreeFolder2 inside Zope 2.9 has been modified to also take this
suppress_events parameter. I'm afraid Plone/AT will have to modify
its _setObject and _delObject implementations to take this optional
parameter too.
As I'm quite new to Zope internals I suppose fixing BTreeFolder2 is
the right thing here (adding the new parameter on both of its
methods). But this change might effect some more products which
have overwritten these methods as CopySupport is a base class of
OFS.Folder and so probably of every folderish type out there.
This was the minimal necessary change to have events work. These
products will have to be modified to work with Zope 2.9, there's no
avoiding it. Fortunately the changes are obvious and minimal. BTW
what BTreeFolder2 are you talking about? BTreeFolder2 has been
included in Zope for a while.
The second change is actually related to your permission work.
First of all I have to thank you for your great work :) But I have
found one nasty thing.
CopySupport had the following security declaration:
__ac_permissions__=(('Copy or Move', (), ('Anonymous', 'Manager',)),)
...
Globals.default__class_init__(CopySource)
which changed into:
security = ClassSecurityInfo()
security.setPermissionDefault(copy_or_move, ('Anonymous', 'Manager'))
...
InitializeClass(CopySource)
That's correct.
Now the InitializeClass call is actually an alias for the former
Globals call, so no change here. But as you wrote yourself, you had
some trouble with the mysterious __ac_permissions format.
Looking at the actual code in App.class_init in the last paragraph
I'm quite sure that the former code did effectivly nothing so far.
The actual setattr call is inside a 'for mname in mnames:' loop
where mnames is the second element of each security tuple - in this
special case the mysterious () which results in not going through
the 'for mname in mnames:' loop at all.
Yes the previous code did nothing, and it just left the
__ac_permissions__ alone.
The new code goes through SecurityInfo.ClassSecurityInfo.apply and,
because there has been a setPermissionDefault, has an available
self.roles dict... Hm I see what's wrong, the SecurityInfo code only
generates permission default roles for permissions which are *also*
mentioned in a method permission setting (the for loop). That code is
wrong, I'll have to fix it.
In the Plone unit tests this results in about 190 failing tests
Could you give me the reference to one or two of these failing tests
so that I can check things are ok after the fix? Is this with a
standard Plone-2.1.1 tarball? Do you run the Plone tests using the
standard Zope method?
as inserting an object into a folder isn't allowed anymore for
normal members because this triggers some code in a Referenceable
class which is based on CopySupport directly, which required no
extra permission so far but now is protected through the 'Copy or
Move' permission.
While I guess that protecting the class with this permission has
been the intent it was not in effect so far. So this should be
either reconsidered or at least noted in the changelog. Changing
this in Archetypes shouldn't be a problem (though I'm very new to
Zope's permissions system).
The class is not protected by copy_or_move, the effect of the
declaration is supposed to be that copy_or_move is by default granted
to Anonymous and Manager. But with my changes because of the
aforementioned bug it's not anymore.
Thanks again for reviewing this! I'll prepare a fix soon.
Florent
--
Florent Guillaume, Nuxeo (Paris, France) Director of RD
+33 1 40 33 71 59 http://nuxeo.com [EMAIL PROTECTED]
___
Zope-Dev maillist - Zope-Dev@zope.org
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
