[Zope-dev] Zope 2 WebDAV and acquisition

[Martin Aspeli]

Martin Aspeli wrote:
Martin Aspeli wrote:
> Martin Aspeli wrote:
>> Hi,
>>
>> In my travails through the ZPublisher and WebDAV, I've come up with 
>> another fun thing.
>>
>> As far as I can tell, traversal via acquisition doesn't make any sense 
>> for a WebDAV request. If I have /foo and /bar, but not /bar/foo, then 
>> traversal to /bar/foo over WebDAV should not acquire /foo and wrap it in 
>> /bar.
>>
>> One reason for this (apart from being utterly confusing) is that it 
>> breaks PUT requests to a NullResource: If I try to PUT to /bar/foo it 
>> should create a new object there, not overwrite /foo.
>>
>> There is actually some convoluted support for detecting this in 
>> ZPublisher.BaseRequest:
>>
>>          # Note - no_acquire_flag is necessary to support
>>          # things like DAV.  We have to make sure
>>          # that the target object is not acquired
>>          # if the request_method is other than GET
>>          # or POST. Otherwise, you could never use
>>          # PUT to add a new object named 'test' if
>>          # an object 'test' existed above it in the
>>          # heirarchy -- you'd always get the
>>          # existing object :(
>>          if (no_acquire_flag and
>>              hasattr(parents[1], 'aq_base') and
>>              not hasattr(parents[1],'__bobo_traverse__')):
>>              if not (hasattr(parents[1].aq_base, entry_name) or
>>                      parents[1].aq_base.has_key(entry_name)):
>>                  raise AttributeError, entry_name
>>
>> This doesn't really work, though. The object is acquired, and then all 
>> it does is to check that the PUT() method object is a true attribute of 
>> the acquired object. But even then, raising AttributeError is wrong.
>>
>> What should happen, I think, is that in 
>> DefaultPublishTraverse.publishTraverse() we should *not* attempt to 
>> acquire for webdav requests. Instead, we should try direct attribute 
>> access and then __getitem__() access (which is the next thing it tries 
>> after getattr() acquisition). This would then allow the folder to return 
>> a NullResource (as OFS.ObjectManager does, for example).
>>
>> Any objections? It's a relatively simple condition in 
>> DfaultPublishTraverse.publishTraverse().
> 
> Actually, I found some code that may be trying to address this (although 
> I think it's probably likely that not attempting to acquire in the first 
> place is better).
> 
> Basically, it allows acquisition during traversal, but when you get to 
> the end of the path, it does this:
> 
>      if (no_acquire_flag and
>          hasattr(object, 'aq_base') and
>          not hasattr(object,'__bobo_traverse__')):
>          if object.aq_parent is not object.aq_inner.aq_parent:
>              from webdav.NullResource import NullResource
>              object = NullResource(parents[-2], object.getId(), 
> self).__of__(parents[-2])
> 
> This would fix the problem, except in my case, the traversed-to object 
> (/foo in the example above) happens to have a __bobo_traverse__(), as do 
> all CMF types, so the third caluse of the first if statement is false.
> 
> I can't see the logic here, though: why would it matter if the acquired 
> object happens to have a __bobo_traverse__?
> 
> Removing that condition fixes it for me. This would be an even simpler 
> bug fix (though I still find it fishy that you can acquire folders 
> through WebDAV).
> 
> Can anyone explain why that condition is there? Otherwise, I'll rip it 
> out. ;-)
> 
> Martin
> 


-- 
Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book
> Martin Aspeli wrote:
>> Hi,
>>
>> In my travails through the ZPublisher and WebDAV, I've come up with 
>> another fun thing.
>>
>> As far as I can tell, traversal via acquisition doesn't make any sense 
>> for a WebDAV request. If I have /foo and /bar, but not /bar/foo, then 
>> traversal to /bar/foo over WebDAV should not acquire /foo and wrap it in 
>> /bar.
>>
>> One reason for this (apart from being utterly confusing) is that it 
>> breaks PUT requests to a NullResource: If I try to PUT to /bar/foo it 
>> should create a new object there, not overwrite /foo.
>>
>> There is actually some convoluted support for detecting this in 
>> ZPublisher.BaseRequest:
>>
>>          # Note - no_acquire_flag is necessary to support
>>          # things like DAV.  We have to make sure
>>          # that the target object is not acquired
>>          # if the request_method is other than GET
>>          # or POST. Otherwise, you could never use
>>          # PUT to add a new object named 'test' if
>>          # an object 'test' existed above it in the
>>          # heirarchy -- you'd always get the
>>          # existing object :(
>>          if (no_acquire_flag and
>>              hasattr(parents[1], 'aq_base') and
>>              not hasattr(parents[1],'__bobo_traverse__')):
>>              if not (hasattr(parents[1].aq_base, entry_name) or
>>                      parents[1].aq_base.has_key(entry_name)):
>>                  raise AttributeError, entry_name
>>
>> This doesn't really work, though. The object is acquired, and then all 
>> it does is to check that the PUT() method object is a true attribute of 
>> the acquired object. But even then, raising AttributeError is wrong.
>>
>> What should happen, I think, is that in 
>> DefaultPublishTraverse.publishTraverse() we should *not* attempt to 
>> acquire for webdav requests. Instead, we should try direct attribute 
>> access and then __getitem__() access (which is the next thing it tries 
>> after getattr() acquisition). This would then allow the folder to return 
>> a NullResource (as OFS.ObjectManager does, for example).
>>
>> Any objections? It's a relatively simple condition in 
>> DfaultPublishTraverse.publishTraverse().
> 
> Actually, I found some code that may be trying to address this (although 
> I think it's probably likely that not attempting to acquire in the first 
> place is better).
> 
> Basically, it allows acquisition during traversal, but when you get to 
> the end of the path, it does this:
> 
>      if (no_acquire_flag and
>          hasattr(object, 'aq_base') and
>          not hasattr(object,'__bobo_traverse__')):
>          if object.aq_parent is not object.aq_inner.aq_parent:
>              from webdav.NullResource import NullResource
>              object = NullResource(parents[-2], object.getId(), 
> self).__of__(parents[-2])
> 
> This would fix the problem, except in my case, the traversed-to object 
> (/foo in the example above) happens to have a __bobo_traverse__(), as do 
> all CMF types, so the third caluse of the first if statement is false.
> 
> I can't see the logic here, though: why would it matter if the acquired 
> object happens to have a __bobo_traverse__?
> 
> Removing that condition fixes it for me. This would be an even simpler 
> bug fix (though I still find it fishy that you can acquire folders 
> through WebDAV).
> 
> Can anyone explain why that condition is there? Otherwise, I'll rip it 
> out. ;-)

One more thing to note: __bobo_traverse__() is eventually called on the 
(wrongly acquired) object, to traverse to the PUT() HTTP verb handler 
method.

I suppose it's possible it could want to do something wildly different, 
thus making the "don't acquire" condition that returns a NullResource 
above incorrect but this still seems very convoluted.

It would need to do a check like "if aq_parent(self) is not 
aq_parent(aq_inner(self)) and is_webdav_request: return 
NullResource(aq_parent(self), self.getId(), request)" even if the name 
is actually available on the object (in this case, the PUT() method).

Demanding that everyone that implements a __bobo_traverse__ does this 
seems really onerous and error prone. I can't see any situations where 
you'd want to be able to do this on a resource that was acquired from 
somewhere other than its containment parent.

Martin

-- 
Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See http://martinaspeli.net/plone-book