#18 — adding a file throws exception (NameError, 'targetpath') p.a.b svn r26015

State Tested and confirmed closed
Version: 1.0
Area Functionality
Issue type Bug
Severity Medium
Submitted by David Hostetler
Submitted on Mar 19, 2009
Responsible Andreas Zeidler
Target release: 1.0
Traceback (innermost last):
  Module ZPublisher.Publish, line 125, in publish
  Module Zope2.App.startup, line 238, in commit
  Module transaction._manager, line 93, in commit
  Module transaction._transaction, line 325, in commit
  Module transaction._transaction, line 427, in _commitResources
  Module ZODB.Connection, line 744, in tpc_vote
  Module ZEO.ClientStorage, line 1004, in tpc_vote
  Module ZEO.ServerStub, line 262, in vote
  Module ZEO.zrpc.connection, line 699, in call
NameError: global name 'targetpath' is not defined


That gets thrown on the first attempt to create a new file on a virgin plone instance, with no products installed besides plone.app.imaging (1.0a2) and plone.app.blob (1.0b3, r26015). Plone itself is v3.2.1

Here's some more platform info:

    * Plone 3.2.1
    * CMF 2.1.2
    * Zope (Zope 2.10.7-final, python 2.4.5, linux2)
    * Python 2.4.5 (#1, Feb 24 2009, 14:50:29) [GCC 4.1.2 (Gentoo 4.1.2 p1.1)]
    * PIL 1.1.6

The buildout.cfg that was used is attached.
Steps to reproduce:
- create a clean instance using the attached buildout.cfg
- start the cluster (it will auto-create the plone site as /Plone)
- browse to the root ZMI (:8280/manage)
- login (as admin)
- navigate to /Plone/portal_quickinstaller
- install plone.app.imaging and plone.app.blob
- browse to the plone home page (:8280/Plone)
- use 'add->file' and try to create a file object, it will throw the exception shown above when you click 'save'.
- insterestingly, the file blob will get created in var/blobstorage/0X00/...../<blobname>.tmp, but the corresponding ATFile object will not exist.
Added by David Hostetler on Mar 20, 2009 04:32 PM
Attached:
buildout.cfg — application/octet-stream, 0 Kb
Added by Andreas Zeidler on Mar 22, 2009 02:59 PM
Issue state: UnconfirmedConfirmed
Responsible manager: (UNASSIGNED)witsch
Target release: None1.0
hi david,

i've upgraded the development buildout for `plone.app.blob` to use plone 3.2.2, but couldn't reproduce the issue. the tests are passing and i could also create and edit blob-based file content when doing some click-testing.

unfortunately, the `buildout.cfg` you tried to provide is empty, too — perhaps you could try to upload it again?

cheers,


andi
Added by David Hostetler on Mar 22, 2009 05:53 PM
I'll re-upload the buildout.cfg tomorrow (it's at work and I don't have access to it today). Sorry I didn't notice it wasn't uploaded successfully.

Note that my attempts were with plone 3.2.1, and you mentioned that you tested with 3.2.2. I don't have any versions pinned in the buildout, so are you getting 3.2.2 manually, or is that what the recipe will pull now? I'll try 3.2.2 on Monday as well.

Thanks! Let me know if you can think of any other info that will help.
Added by Andreas Zeidler on Mar 23, 2009 12:41 AM
i can't imagine how 3.2.1 vs 3.2.2 would make any difference regarding blobs. the changes are pretty minimal — see http://plone.org/products/plone/releases/3.2.2. i was already using it, since i knew it's available. it's just not officially released yet (because the windows installer is still missing).

as for getting it, plone 3.2.x doesn't use the `plone.recipe.plone` recipe anymore, you'll have to change the `extends` and `find-links` settings in your `buildout.cfg` to get a different version. but like i said, it shouldn't make a difference, and the tests are also passing on 3.2.1.

the same goes for 3.1.7 vs 3.2.1, btw. the first 3.2 release (3.2.0 was skipped since `NuPlone` got accidentally dropped) was merely a re-packaged version of the current 3.1.x, at least almost. :) there were hardly any code changes, so the chances of something breaking blob support should be pretty slim — in theory. but let's see what your `buildout.cfg` brings up... :)
Added by David Hostetler on Mar 23, 2009 01:29 PM
Attached:
buildout.cfg — application/octet-stream, 10 Kb
Added by David Hostetler on Mar 23, 2009 08:53 PM
I'm not sure how a plain 3.2.1 install works for you. It actually first fails because the permissions on the (automatically created) blobstorage won't allow the zeo clients to write to it. I had to manually change var/blobstorage/tmp to be chmod 770 and chown plone:zeo. Otherwise, the first attempt to create a blob file threw a permissions error because the zeo client couldn't write to blobstorage/tmp (since it runs as 'plone' , and not 'zeo').

And then the next attempt triggers the 'targetpath' exception previously described.

I used the Plone-3.2.1r3 Unified Installer tarball directly from plone.org, ran 'install.sh zeo' to get this thing bootstrapped.

I'm downloading the 3.2.2 tarball now and will try that from scratch in its own sandbox.
Added by David Hostetler on Mar 23, 2009 08:57 PM
I'm beginning to suspect that the directory permissions are exposing the problem.

See line 370 of ZODB/blob.py:

                assert os.path.exists(targetpath)

That's in the context of getPathForOID():

        if create and not os.path.exists(path):
            try:
                os.makedirs(path, 0700)
            except OSError:
                # We might have lost a race. If so, the directory
                # must exist now
                assert os.path.exists(targetpath)
        return path


I'm pretty sure that 'targetpath' is supposed to be just 'path'. Bug. And it's getting exposed because the makedirs() call is failing, triggering the exception handler that apparently isn't exercised in any of the tests (or the 'targetpath' bug would have been exposed).

I'm trying to characterize the permissions expectations now, to see what allows it to work.

Added by David Hostetler on Mar 23, 2009 09:04 PM
Ok, the following change allowed plone.app.blob to function correctly in a zeo buildout:

[client1]
- effective-user = plone
+ effective-user = zeo

That caused the var/blobstorage hierarchy to get chown'd zeo:zeo, and not trigger the bad exception handler in FilesystemHelper.getPathForOID().

I'm guessing that's not a desired requirement: for the zeo client and the zeo server to have to run as the same UID/GID.
Added by Andreas Zeidler on May 11, 2009 02:45 PM
hi david,

the 'targetpath' issue was already fixed for zodb 3.9, but not on the 3.8 branch. i've backported the fix (http://svn.zope.org/?rev=99808&view=rev) now, but i'm not sure how long it'll take until another release is made. i'll try to poke the right people, though. in any case, thanks for debugging this!

apart from that, you're right, of course — the zeo server & client shouldn't necessarily run as the same user. however, these things are a deployment issue and the `plone.app.blob` package cannot really do anything about it. i know some people have been working on a secure buildout configuration for a "blobified" zeo-setup, though, and i will update the documentation accordingly when i find some time for reviewing...

in the meantime you might want to try contacting ken manheimer and/or graham perrin about it.
Added by Andreas Zeidler on May 11, 2009 02:46 PM
Issue state: ConfirmedResolved
that said i think this can be closed now that the originally reported issue is fixed. feel free to poke me again re the docs! ;)
Added by David Hostetler on May 11, 2009 02:53 PM
Issue state: ResolvedTested and confirmed closed
Roger that. Thanks for the resolution and the additional info.

No responses can be added.