jim at zope
Oct 31, 2005, 7:26 AM
Post #4 of 6
(6420 views)
Permalink
|
|
Re: Re: Confusing code in ObjectManager manage_FTPstat
[In reply to]
|
|
Sidnei da Silva wrote:
> On Mon, Oct 31, 2005 at 10:05:45AM -0500, Jim Fulton wrote:
> | Sidnei da Silva wrote:
> | >Found some lovely piece of code deep into the FTP parts of Zope 2 last
> | >saturday, one of them is truely ugly. It's listing the contents of
> | >the current and parent folders for no apparent reason (or at least, it
> | >didn't make sense either to me or Chris McDonough).
> |
> | There's a comment stating the intent.
>
> Yes, but the original code does the check in a truely obscure way, at
> least to me. I've thought of spelling:
>
> # check to see if we are acquiring our objectValues or not
> if not (len(REQUEST.PARENTS) > 1 and
> self.objectValues() == REQUEST.PARENTS[1].objectValues()):
>
> As:
>
> if self.objectValues.im_self is not self:
That might be better.
> However I'm not sure that would be the be the correct
> replacement. There doesn't seem to exist any test for this.
Of course, there's no test for this. We weren't doing automated
testing back then. Feel free to write one.
> | >The code in question is in the ``manage_FTPstat`` method of
> | >``OFS.ObjectManager``. Tracing back the source of this code, it seems
> | >to have been (surprisingly) introduced by Jim Fulton, back in
> | >1999. [1]
> | >
> | >Later on, Amos Latteier changed part of the code (namely
> | >``manage_FTPlist``) to use a slightly more friendlier, yet equally
> | >confusing ``is_acquired`` method. [2]
> | >
> | >I'm now sitting here, trying to make sense of this code and wondering
> | >what was the original intention in the first place. Would anyone have
> | >a clue?
> |
> | I think the comments make this pretyu clear. The intent is to avoid
> | providing listings of acquired objects. In fact, the intent of both
> | bits of code, I think is to prevent FTP access to acquired objects.
>
> Wouldn't that be better implemented by making the same check as it's
> done for WebDAV and set 'maybe_webdav_client' (abusing the name)
> so that it sets the 'no_acquire_flag'?
Possibly.
> | I suspect that there is a clearer way to implement the intent.
>
> I have three suggestions right now:
>
> 1. change manage_FTPstat to use the same thing as manage_FTPlist
> (using App.Common.is_acquired, and possibly rewriting is_acquired
> to a simpler check)
That's an improvement,
> OR
>
> 2. Throw away the `is_acquired` check and replace it by a much
> simpler check
Sounds good, doing so consistently.
> OR
>
> 3. Setting 'maybe_webdav_client' in the request object so the object
> is never acquired for FTP, the same way as it's done for WebDAV.
Possibly. I don't have time to look at that.
Jim
--
Jim Fulton mailto:jim [at] zope Python Powered!
CTO (540) 361-1714 http://www.python.org
Zope Corporation http://www.zope.com http://www.zope.org
_______________________________________________
Zope-Coders mailing list
Zope-Coders [at] zope
http://mail.zope.org/mailman/listinfo/zope-coders
|
