TracAdmin allows attachment to be added to non-existent resource

[Ryan J Ollos]

To reproduce:

• trac-admin $ENV attachment add wiki:SomeNonExistentPage file
• Navigate to /attachment/wiki/SomeNonExistentPage and the attachment will be found

• Navigate to /wiki/SomeNonExistentPage to find the following:

Even worse, an attachment could be created for a non-existent realm:

$ trac-admin $ENV attachment add some:none file1

When attempting to view the attachment at the path /attachment/some/none/file1, the permission check will fail: ATTACHMENT_VIEW privileges are required to perform this operation on Attachment 'file1' in some:none. You don't have the required permissions.. However, inspection of the database or execution of the trac-admin attachment list command shows that the attachment exists.

Proposed changes can be found in log:rjollos.git:t11397. Besides the fix for the issue in this ticket, there is one additional change proposed:

• Raise an error if navigating to an attachment page for which the parent resource doesn't exist.

[Ryan J Ollos]

Committed to 1.0-stable in [12330] and merged to trunk in [12331].

[Jun Omae]

The test_attachment_add_nonexistent_resource fails on Windows.

FAIL: test_attachment_add_nonexistent_resource (trac.admin.tests.console.TracadminTestCase)
Tests the 'attachment add' command in trac-admin, on a non-existent
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\usr\src\trac\trac.git\trac\admin\tests\console.py", line 257, in test_attachment_add_nonexistent_resource
    self.assertEqual(self.expected_results[test_name], output)
  File "C:\usr\src\trac\trac.git\trac\admin\tests\console.py", line 159, in assertEqual
    output, diff()))
AssertionError: u"ResourceNotFound: NonExistentPage doesn't exist, can't create attachment\n" != u"IOError: [Errno 13] Permission denied: u'c:\\\\docume~1\\\\jun66j5\\\\locals~1\\\\temp\\\\tmp8snnqg'\n"
--- expected
+++ actual
@@ -1,2 +1,2 @@
-ResourceNotFound: NonExistentPage doesn't exist, can't create attachment
+IOError: [Errno 13] Permission denied: u'c:\\docume~1\\jun66j5\\locals~1\\temp\\tmp8snnqg'

----------------------------------------------------------------------
Ran 1437 tests in 123.453s

FAILED (failures=1)

In Windows, NamedTemporaryFile create a temporary file with os.O_TEMPORARY flag. The temporary file with the flag cannot be opened again. It leads the failure.

Python 2.5.4 (r254:67916, Dec 23 2008, 15:10:54) [MSC v.1310 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> fd = os.open('C:/tmp/blah.txt',
...              os.O_RDWR | os.O_CREAT | os.O_EXCL | os.O_BINARY | os.O_TEMPORARY,
...              0600)
>>> fd
3
>>> open('C:/tmp/blah.txt')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
IOError: [Errno 13] Permission denied: 'C:/tmp/blah.txt'
>>>

It would be simple to use __file__ rather than tempfile.

trac/admin/tests/console.py

@@ -250 +250 @@
         """Tests the 'attachment add' command in trac-admin, on a non-existent
         resource."""
         test_name = sys._getframe().f_code.co_name
-        file = tempfile.NamedTemporaryFile()
         rv, output = self._execute('attachment add wiki:NonExistentPage %s'
-                                   % file.name)
+                                   % __file__)
         self.assertEqual(2, rv)
         self.assertEqual(self.expected_results[test_name], output)

[Ryan J Ollos]

Thanks. Committed to 1.0-stable in [12390:12391], and merged to trunk in [12392].

[Jun Omae]

After r12330, unit tests failed with PostgreSQL. Also the same occur on r12494.

$ git log --max-count=1 HEAD | grep git-svn-id
    git-svn-id: http://trac.edgewall.org/intertrac/log:/branches/1.0-stable@12330 af82e41b-90c4-0310-8c96-b1721e28e2e2
$ make python=26 db=postgres unit-test
Python version: Python 2.6.6
figleaf:
coverage: /home/jun66j5/bin/coverage
PYTHONPATH=.:
TRAC_TEST_DB_URI=postgres://tracuser:password@localhost:5432/tractest?schema=tractest
server-options= -p 3000 -a '*,/home/jun66j5/src/trac-htdigest.txt,auth' -s -r -e
python setup.py egg_info
running egg_info
writing requirements to Trac.egg-info/requires.txt
writing Trac.egg-info/PKG-INFO
writing top-level names to Trac.egg-info/top_level.txt
writing dependency_links to Trac.egg-info/dependency_links.txt
writing entry points to Trac.egg-info/entry_points.txt
reading manifest file 'Trac.egg-info/SOURCES.txt'
writing manifest file 'Trac.egg-info/SOURCES.txt'
python ./trac/test.py --skip-functional-tests
/home/jun66j5/venv/py26/lib/python2.6/site-packages/twill/other_packages/_mechanize_dist/_auth.py:14: DeprecationWarning: the md5 module is deprecated; use hashlib instead
  import re, base64, urlparse, posixpath, md5, sha, sys, copy
/home/jun66j5/venv/py26/lib/python2.6/site-packages/twill/other_packages/_mechanize_dist/_auth.py:14: DeprecationWarning: the sha module is deprecated; use the hashlib module instead
  import re, base64, urlparse, posixpath, md5, sha, sys, copy
EEEEEEEEEEEEE...................................................................
........................EEEEEEEE................................................
................................................................................
................................................................................
................................................................................
................................................................................
................................................................................
................................................................................
................................................................................
................................................................................
................................................................................
................................................................................
................................................................................
................................................................................
................................................................................
................................................................................
................................................................................
................................................................................
..........................
======================================================================
ERROR: test_delete (trac.tests.attachment.AttachmentTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jun66j5/src/trac.git/trac/tests/attachment.py", line 65, in setUp
    self.env.db_transaction("INSERT INTO wiki (name) VALUES ('WikiStart')")
  File "/home/jun66j5/src/trac.git/trac/db/api.py", line 122, in execute
    return db.execute(query, params)
  File "/home/jun66j5/src/trac.git/trac/db/util.py", line 121, in execute
    cursor.execute(query, params)
  File "/home/jun66j5/src/trac.git/trac/db/util.py", line 56, in execute
    r = self.cursor.execute(sql)
IntegrityError: null value in column "version" violates not-null constraint

...

======================================================================
ERROR: Test mailto: not obfuscated, unlike plain email
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jun66j5/src/trac.git/trac/wiki/tests/formatter.py", line 190, in setUp
    wiki.save('joe', 'Entry page', '::1', datetime.now(utc))
  File "/home/jun66j5/src/trac.git/trac/wiki/model.py", line 143, in save
    self.readonly))
  File "/home/jun66j5/src/trac.git/trac/db/util.py", line 121, in execute
    cursor.execute(query, params)
  File "/home/jun66j5/src/trac.git/trac/db/util.py", line 54, in execute
    r = self.cursor.execute(sql_escape_percent(sql), args)
IntegrityError: duplicate key value violates unique constraint "wiki_pk"


----------------------------------------------------------------------
Ran 1466 tests in 237.282s

FAILED (errors=21)
make: *** [unit-test] Error 1

[Jun Omae]

Proposed fix can be found in jomae.git@t11397.1.

[Ryan J Ollos]

The changes look good. Tested with Python 2.7; PostgreSQL, MySQL and SQLite on Ubuntu 13.04.

Thank you for fixing this. I've been sloppy by not running the unit tests for all databases, but with the makefile it is so easy that there is really no excuse.

[Jun Omae]

Thanks for testing. I tested with Python 2.5-2.6, SQLite 3.3.6, PostgreSQL 8.1.23 and MySQL 5.0.95. No worries. I had not run the tests for all databases, also.

Committed and merged in [12498-12499].