Merge lp:~jderose/dmedia/filestore-tmp-api into lp:dmedia

Proposed by Jason Gerard DeRose
Status: Merged
Approved by: Jason Gerard DeRose
Approved revision: 170
Merged at revision: 156
Proposed branch: lp:~jderose/dmedia/filestore-tmp-api
Merge into: lp:dmedia
Diff against target: 582 lines (+250/-66)
4 files modified
dmedia/downloader.py (+2/-2)
dmedia/filestore.py (+119/-42)
dmedia/tests/test_downloader.py (+1/-1)
dmedia/tests/test_filestore.py (+128/-21)
To merge this branch: bzr merge lp:~jderose/dmedia/filestore-tmp-api
Reviewer Review Type Date Requested Status
James Raymond Approve
dmedia Dev Pending
Review via email: mp+49495@code.launchpad.net

Description of the change

Splits FileStore.finalize_transfer() into two simpler methods:

FileStore.tmp_move(tmp_fp, chash, ext) - only moves tmp_fp to canonical location, assumes higher level code has already computed/verified content hash

FileStore.tmp_verify_move(chash, ext) - basically same as finalize_transfer() except uses tmp_move() to do actually move

To fit these naming conventions, I also renamed:

FileStore.reltemp() => FileStore.reltmp()
FileStore.temp() => FileStore.tmp()

Meaning to do that for a while, so good excuse.

Probably easier to read the docstrings here - http://bazaar.launchpad.net/~jderose/dmedia/filestore-tmp-api/view/head:/dmedia/filestore.py#L625

To post a comment you must log in.
Revision history for this message
James Raymond (jamesmr) wrote :

Looks decent :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'dmedia/downloader.py'
--- dmedia/downloader.py 2011-02-11 16:55:18 +0000
+++ dmedia/downloader.py 2011-02-12 11:14:54 +0000
@@ -172,12 +172,12 @@
172 self.ext = ext172 self.ext = ext
173173
174 def get_tmp(self):174 def get_tmp(self):
175 tmp = self.fs.temp(self.chash, self.ext, create=True)175 tmp = self.fs.tmp(self.chash, self.ext, create=True)
176 log.debug('Writting file to %r', tmp)176 log.debug('Writting file to %r', tmp)
177 return tmp177 return tmp
178178
179 def finalize(self):179 def finalize(self):
180 dst = self.fs.finalize_transfer(self.chash, self.ext)180 dst = self.fs.tmp_verify_move(self.chash, self.ext)
181 log.debug('Canonical name is %r', dst)181 log.debug('Canonical name is %r', dst)
182 return dst182 return dst
183183
184184
=== modified file 'dmedia/filestore.py'
--- dmedia/filestore.py 2011-02-11 17:22:08 +0000
+++ dmedia/filestore.py 2011-02-12 11:14:54 +0000
@@ -39,6 +39,7 @@
3939
40import os40import os
41from os import path41from os import path
42import stat
42import tempfile43import tempfile
43from hashlib import sha1 as HASH44from hashlib import sha1 as HASH
44from base64 import b32encode, b32decode45from base64 import b32encode, b32decode
@@ -278,8 +279,6 @@
278 if not chunk:279 if not chunk:
279 break280 break
280 self.thread.join()281 self.thread.join()
281 if self.dst_fp is not None:
282 os.fchmod(self.dst_fp.fileno(), 0o444)
283 return b32encode(self.h.digest())282 return b32encode(self.h.digest())
284283
285284
@@ -450,18 +449,18 @@
450 return (dname, fname)449 return (dname, fname)
451450
452 @staticmethod451 @staticmethod
453 def reltemp(chash, ext=None):452 def reltmp(chash, ext=None):
454 """453 """
455 Relative path of temporary file with *chash*, ending with *ext*.454 Relative path of temporary file with *chash*, ending with *ext*.
456455
457 For example:456 For example:
458457
459 >>> FileStore.reltemp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW')458 >>> FileStore.reltmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW')
460 ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW')459 ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW')
461460
462 Or with the file extension *ext*:461 Or with the file extension *ext*:
463462
464 >>> FileStore.reltemp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', ext='mov')463 >>> FileStore.reltmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', ext='mov')
465 ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.mov')464 ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.mov')
466465
467 Also see `FileStore.relpath()`.466 Also see `FileStore.relpath()`.
@@ -472,6 +471,9 @@
472 return (TRANSFERS_DIR, chash)471 return (TRANSFERS_DIR, chash)
473472
474 def check_path(self, pathname):473 def check_path(self, pathname):
474 """
475 Verify that *pathname* in inside this filestore base directory.
476 """
475 abspath = path.abspath(pathname)477 abspath = path.abspath(pathname)
476 if abspath.startswith(self.base + os.sep):478 if abspath.startswith(self.base + os.sep):
477 return abspath479 return abspath
@@ -574,7 +576,7 @@
574 self.create_parent(filename)576 self.create_parent(filename)
575 return filename577 return filename
576578
577 def temp(self, chash, ext=None, create=False):579 def tmp(self, chash, ext=None, create=False):
578 """580 """
579 Returns path of temporary file with *chash*, ending with *ext*.581 Returns path of temporary file with *chash*, ending with *ext*.
580582
@@ -582,26 +584,26 @@
582 in which case the content-hash is already known. For example:584 in which case the content-hash is already known. For example:
583585
584 >>> fs = FileStore()586 >>> fs = FileStore()
585 >>> fs.temp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW') #doctest: +ELLIPSIS587 >>> fs.tmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW') #doctest: +ELLIPSIS
586 '/tmp/store.../transfers/NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'588 '/tmp/store.../transfers/NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'
587589
588590
589 Or with a file extension:591 Or with a file extension:
590592
591 >>> fs.temp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', 'txt') #doctest: +ELLIPSIS593 >>> fs.tmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', 'txt') #doctest: +ELLIPSIS
592 '/tmp/store.../transfers/NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.txt'594 '/tmp/store.../transfers/NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.txt'
593595
594596
595 If called with ``create=True``, the parent directory is created with597 If called with ``create=True``, the parent directory is created with
596 `FileStore.create_parent()`.598 `FileStore.create_parent()`.
597 """599 """
598 filename = self.join(*self.reltemp(chash, ext))600 filename = self.join(*self.reltmp(chash, ext))
599 if create:601 if create:
600 self.create_parent(filename)602 self.create_parent(filename)
601 return filename603 return filename
602604
603 def allocate_for_transfer(self, size, chash, ext=None):605 def allocate_for_transfer(self, size, chash, ext=None):
604 filename = self.temp(chash, ext, create=True)606 filename = self.tmp(chash, ext, create=True)
605 fallocate(size, filename)607 fallocate(size, filename)
606 try:608 try:
607 fp = open(filename, 'r+b')609 fp = open(filename, 'r+b')
@@ -620,14 +622,99 @@
620 fallocate(size, filename)622 fallocate(size, filename)
621 return open(filename, 'r+b')623 return open(filename, 'r+b')
622624
623 def finalize_transfer(self, chash, ext=None):625 def tmp_move(self, tmp_fp, chash, ext=None):
624 """626 """
625 Move canonically named temporary file to its final canonical location.627 Move temporary file into its canonical location.
628
629 This method will securely and atomically move a temporary file into its
630 canonical location.
631
632 For example:
633
634 >>> fs = FileStore()
635 >>> tmp_fp = open(fs.join('foo.mov'), 'wb')
636 >>> chash = 'ZR765XWSF6S7JQHLUI4GCG5BHGPE252O'
637 >>> fs.tmp_move(tmp_fp, chash, 'mov') #doctest: +ELLIPSIS
638 '/tmp/store.../ZR/765XWSF6S7JQHLUI4GCG5BHGPE252O.mov'
639
640
641 Note, however, that this method does *not* verify the content hash of
642 the temporary file! This is by design as many operations will compute
643 the content hash as they write to the temporary file. Other operations
644 should use `FileStore.tmp_verify_move()` to verify and move in one step.
645
646 Regardless, the full content hash should have been verified prior to
647 calling this method. To ensure the content is not modified, operations
648 must take these steps:
649
650 1. Open *tmp_fp* and keep it open, thereby retaining a lock on the
651 file
652
653 2. Compute the full content hash, which can be done as content is
654 written to *tmp_fp* (open in mode ``'r+b'`` to resume a transfer,
655 but hash of previously transfered leaves must still be verified)
656
657 3. With *tmp_fp* still open, move the temporary file into its
658 canonical location using this method.
659
660 As a simple locking mechanism, this method takes an open ``file`` rather
661 than a filename, thereby preventing the file from being modified during
662 the move. A ``ValueError`` is raised if *tmp_fp* is already closed.
663
664 For portability reasons, this method requires that *tmp_fp* be opened in
665 a binary mode: ``'rb'``, ``'wb'``, or ``'r+b'``. A ``ValueError`` is
666 raised if opened in any other mode.
667
668 For security reasons, this method will only move a temporary file
669 located within the ``FileStore.base`` directory or a subdirectory
670 thereof. If an attempt is made to move a file from outside the store,
671 `FileStoreTraversal` is raised. See `FileStore.check_path()`.
672
673 Just prior to moving the file, a call to ``os.fchmod()`` is made to set
674 read-only permissions (0444). After the move, *tmp_fp* is closed.
675
676 If the canonical file already exists, `DuplicateFile` is raised.
677
678 The return value is the absolute path of the canonical file.
679
680 :param tmp_fp: a ``file`` instance created with ``open()``
681 :param chash: base32-encoded content-hash
682 :param ext: normalized lowercase file extension, eg ``'mov'``
683 """
684 # Validate tmp_fp:
685 if not isinstance(tmp_fp, file):
686 raise TypeError(
687 TYPE_ERROR % ('tmp_fp', file, type(tmp_fp), tmp_fp)
688 )
689 if tmp_fp.mode not in ('rb', 'wb', 'r+b'):
690 raise ValueError(
691 "tmp_fp: mode must be 'rb', 'wb', or 'r+b'; got %r" % tmp_fp.mode
692 )
693 if tmp_fp.closed:
694 raise ValueError('tmp_fp is closed, must be open: %r' % tmp_fp.name)
695 self.check_path(tmp_fp.name)
696
697 # Get canonical name, check for duplicate:
698 dst = self.path(chash, ext, create=True)
699 if path.exists(dst):
700 raise DuplicateFile(chash=chash, src=tmp_fp.name, dst=dst)
701
702 # Set file to read-only (0444) and move into canonical location
703 os.fchmod(tmp_fp.fileno(), stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH)
704 os.rename(tmp_fp.name, dst)
705 tmp_fp.close()
706
707 # Return canonical filename:
708 return dst
709
710 def tmp_verify_move(self, chash, ext=None):
711 """
712 Verify temporary file, then move into its canonical location.
626713
627 This method will check the content hash of the canonically-named714 This method will check the content hash of the canonically-named
628 temporary file with content hash *chash* and extension *ext*. If the715 temporary file with content hash *chash* and extension *ext*. If the
629 content hash is correct, it will do an ``os.fchmod()`` to set read-only716 content hash is correct, this method will then move the temporary file
630 permissions, and then rename the file into its canonical location.717 into its canonical location using `FileStore.tmp_move()`.
631718
632 If the content hash is incorrect, `IntegrityError` is raised. If the719 If the content hash is incorrect, `IntegrityError` is raised. If the
633 canonical file already exists, `DuplicateFile` is raised. Lastly, if720 canonical file already exists, `DuplicateFile` is raised. Lastly, if
@@ -639,7 +726,7 @@
639 temporary file name, like this:726 temporary file name, like this:
640727
641 >>> fs = FileStore()728 >>> fs = FileStore()
642 >>> tmp = fs.temp('ZR765XWSF6S7JQHLUI4GCG5BHGPE252O', 'mov', create=True)729 >>> tmp = fs.tmp('ZR765XWSF6S7JQHLUI4GCG5BHGPE252O', 'mov', create=True)
643 >>> tmp #doctest: +ELLIPSIS730 >>> tmp #doctest: +ELLIPSIS
644 '/tmp/store.../transfers/ZR765XWSF6S7JQHLUI4GCG5BHGPE252O.mov'731 '/tmp/store.../transfers/ZR765XWSF6S7JQHLUI4GCG5BHGPE252O.mov'
645732
@@ -662,33 +749,23 @@
662 Finally, the downloader will move the temporary file into its canonical749 Finally, the downloader will move the temporary file into its canonical
663 location:750 location:
664751
665 >>> dst = fs.finalize_transfer('ZR765XWSF6S7JQHLUI4GCG5BHGPE252O', 'mov')752 >>> dst = fs.tmp_verify_move('ZR765XWSF6S7JQHLUI4GCG5BHGPE252O', 'mov')
666 >>> dst #doctest: +ELLIPSIS753 >>> dst #doctest: +ELLIPSIS
667 '/tmp/store.../ZR/765XWSF6S7JQHLUI4GCG5BHGPE252O.mov'754 '/tmp/store.../ZR/765XWSF6S7JQHLUI4GCG5BHGPE252O.mov'
668755
669756
670 Note above that this method returns the full path of the canonically757 The return value is the absolute path of the canonical file.
671 named file.758
759 :param chash: base32-encoded content-hash
760 :param ext: normalized lowercase file extension, eg ``'mov'``
672 """761 """
673 # Open temporary file and check content hash:762 tmp = self.tmp(chash, ext)
674 tmp = self.temp(chash, ext)
675 tmp_fp = open(tmp, 'rb')763 tmp_fp = open(tmp, 'rb')
676 h = HashList(tmp_fp)764 h = HashList(tmp_fp)
677 got = h.run()765 got = h.run()
678 if got != chash:766 if got != chash:
679 raise IntegrityError(got=got, expected=chash, filename=tmp_fp.name)767 raise IntegrityError(got=got, expected=chash, filename=tmp_fp.name)
680768 return self.tmp_move(tmp_fp, chash, ext)
681 # Get canonical name, check for duplicate:
682 dst = self.path(chash, ext, create=True)
683 if path.exists(dst):
684 raise DuplicateFile(chash=chash, src=tmp_fp.name, dst=dst)
685
686 # Set file to read-only and rename into canonical location
687 os.fchmod(tmp_fp.fileno(), 0o444)
688 os.rename(tmp_fp.name, dst)
689
690 # Return canonical filename:
691 return dst
692769
693 def import_file(self, src_fp, ext=None):770 def import_file(self, src_fp, ext=None):
694 """771 """
@@ -696,8 +773,8 @@
696773
697 The method will compute the content-hash of *src_fp* as it copies it to774 The method will compute the content-hash of *src_fp* as it copies it to
698 a temporary file within this store. Once the copying is complete, the775 a temporary file within this store. Once the copying is complete, the
699 file will be renamed to its canonical location in the store, thus776 temporary file will be moved to its canonical location using
700 ensuring an atomic operation.777 `FileStore.tmp_move()`.
701778
702 A `DuplicatedFile` exception will be raised if the file already exists779 A `DuplicatedFile` exception will be raised if the file already exists
703 in this store.780 in this store.
@@ -708,15 +785,15 @@
708785
709 Note that *src_fp* must have been opened in ``'rb'`` mode.786 Note that *src_fp* must have been opened in ``'rb'`` mode.
710787
711 :param src_fp: A ``file`` instance created with ``open()``788 :param src_fp: a ``file`` instance created with ``open()``
712 :param ext: The file's extension, e.g., ``'ogv'``789 :param ext: normalized lowercase file extension, eg ``'mov'``
713 """790 """
714 size = os.fstat(src_fp.fileno()).st_size791 size = os.fstat(src_fp.fileno()).st_size
715 tmp_fp = self.allocate_for_import(size, ext=ext)792 tmp_fp = self.allocate_for_import(size, ext)
716 h = HashList(src_fp, tmp_fp)793 h = HashList(src_fp, tmp_fp)
717 chash = h.run()794 chash = h.run()
718 dst = self.path(chash, ext, create=True)795 try:
719 if path.exists(dst):796 self.tmp_move(tmp_fp, chash, ext)
720 raise DuplicateFile(chash=chash, src=src_fp.name, dst=dst)797 except DuplicateFile as e:
721 os.rename(tmp_fp.name, dst)798 raise DuplicateFile(src=src_fp.name, dst=e.dst, chash=e.chash)
722 return (chash, h.leaves)799 return (chash, h.leaves)
723800
=== modified file 'dmedia/tests/test_downloader.py'
--- dmedia/tests/test_downloader.py 2011-02-11 15:45:43 +0000
+++ dmedia/tests/test_downloader.py 2011-02-12 11:14:54 +0000
@@ -212,7 +212,7 @@
212 self.assertFalse(path.exists(dst))212 self.assertFalse(path.exists(dst))
213213
214 # Test when transfers/ exists but file does not:214 # Test when transfers/ exists but file does not:
215 self.assertEqual(fs.temp(mov_hash, 'mov', create=True), src)215 self.assertEqual(fs.tmp(mov_hash, 'mov', create=True), src)
216 self.assertTrue(path.isdir(src_d))216 self.assertTrue(path.isdir(src_d))
217 e = raises(IOError, inst.finalize)217 e = raises(IOError, inst.finalize)
218 self.assertFalse(path.exists(src))218 self.assertFalse(path.exists(src))
219219
=== modified file 'dmedia/tests/test_filestore.py'
--- dmedia/tests/test_filestore.py 2011-02-11 01:50:20 +0000
+++ dmedia/tests/test_filestore.py 2011-02-12 11:14:54 +0000
@@ -26,6 +26,7 @@
2626
27import os27import os
28from os import path28from os import path
29import stat
29from hashlib import sha130from hashlib import sha1
30from base64 import b32encode, b32decode31from base64 import b32encode, b32decode
31import shutil32import shutil
@@ -530,24 +531,24 @@
530 'ext %r does not match pattern %r' % (bad, EXT_PAT)531 'ext %r does not match pattern %r' % (bad, EXT_PAT)
531 )532 )
532533
533 def test_reltemp(self):534 def test_reltmp(self):
534 self.assertEqual(535 self.assertEqual(
535 self.klass.reltemp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'),536 self.klass.reltmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'),
536 ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW')537 ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW')
537 )538 )
538 self.assertEqual(539 self.assertEqual(
539 self.klass.reltemp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', ext='ogv'),540 self.klass.reltmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', ext='ogv'),
540 ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.ogv')541 ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.ogv')
541 )542 )
542543
543 # Test to make sure hashes are getting checked with safe_b32():544 # Test to make sure hashes are getting checked with safe_b32():
544 bad = 'NWBNVXVK5..GIOW7MYR4K3KA5K22W7NW'545 bad = 'NWBNVXVK5..GIOW7MYR4K3KA5K22W7NW'
545 e = raises(ValueError, self.klass.reltemp, bad)546 e = raises(ValueError, self.klass.reltmp, bad)
546 self.assertEqual(547 self.assertEqual(
547 str(e),548 str(e),
548 'b32: cannot b32decode %r: Non-base32 digit found' % bad549 'b32: cannot b32decode %r: Non-base32 digit found' % bad
549 )550 )
550 e = raises(ValueError, self.klass.reltemp, bad, ext='ogv')551 e = raises(ValueError, self.klass.reltmp, bad, ext='ogv')
551 self.assertEqual(552 self.assertEqual(
552 str(e),553 str(e),
553 'b32: cannot b32decode %r: Non-base32 digit found' % bad554 'b32: cannot b32decode %r: Non-base32 digit found' % bad
@@ -556,7 +557,7 @@
556 # Test to make sure ext is getting checked with safe_ext():557 # Test to make sure ext is getting checked with safe_ext():
557 chash = 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'558 chash = 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'
558 bad = '/../../../.ssh/id_pub'559 bad = '/../../../.ssh/id_pub'
559 e = raises(ValueError, self.klass.reltemp, chash, bad)560 e = raises(ValueError, self.klass.reltmp, chash, bad)
560 self.assertEqual(561 self.assertEqual(
561 str(e),562 str(e),
562 'ext %r does not match pattern %r' % (bad, EXT_PAT)563 'ext %r does not match pattern %r' % (bad, EXT_PAT)
@@ -750,27 +751,27 @@
750 self.assertFalse(path.exists(f))751 self.assertFalse(path.exists(f))
751 self.assertTrue(path.isdir(d))752 self.assertTrue(path.isdir(d))
752753
753 def test_temp(self):754 def test_tmp(self):
754 tmp = TempDir()755 tmp = TempDir()
755 inst = self.klass(tmp.path)756 inst = self.klass(tmp.path)
756757
757 self.assertEqual(758 self.assertEqual(
758 inst.temp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'),759 inst.tmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'),
759 tmp.join('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW')760 tmp.join('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW')
760 )761 )
761 self.assertEqual(762 self.assertEqual(
762 inst.temp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', ext='ogv'),763 inst.tmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', ext='ogv'),
763 tmp.join('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.ogv')764 tmp.join('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.ogv')
764 )765 )
765766
766 # Test to make sure hashes are getting checked with safe_b32():767 # Test to make sure hashes are getting checked with safe_b32():
767 bad = 'NWBNVXVK5..GIOW7MYR4K3KA5K22W7NW'768 bad = 'NWBNVXVK5..GIOW7MYR4K3KA5K22W7NW'
768 e = raises(ValueError, inst.temp, bad)769 e = raises(ValueError, inst.tmp, bad)
769 self.assertEqual(770 self.assertEqual(
770 str(e),771 str(e),
771 'b32: cannot b32decode %r: Non-base32 digit found' % bad772 'b32: cannot b32decode %r: Non-base32 digit found' % bad
772 )773 )
773 e = raises(ValueError, inst.temp, bad, ext='ogv')774 e = raises(ValueError, inst.tmp, bad, ext='ogv')
774 self.assertEqual(775 self.assertEqual(
775 str(e),776 str(e),
776 'b32: cannot b32decode %r: Non-base32 digit found' % bad777 'b32: cannot b32decode %r: Non-base32 digit found' % bad
@@ -779,7 +780,7 @@
779 # Test to make sure ext is getting checked with safe_ext():780 # Test to make sure ext is getting checked with safe_ext():
780 chash = 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'781 chash = 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'
781 bad = '/../../../.ssh/id_pub'782 bad = '/../../../.ssh/id_pub'
782 e = raises(ValueError, inst.temp, chash, bad)783 e = raises(ValueError, inst.tmp, chash, bad)
783 self.assertEqual(784 self.assertEqual(
784 str(e),785 str(e),
785 'ext %r does not match pattern %r' % (bad, EXT_PAT)786 'ext %r does not match pattern %r' % (bad, EXT_PAT)
@@ -794,13 +795,13 @@
794 self.assertFalse(path.exists(f))795 self.assertFalse(path.exists(f))
795 self.assertFalse(path.exists(d))796 self.assertFalse(path.exists(d))
796 self.assertEqual(797 self.assertEqual(
797 inst.temp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'),798 inst.tmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'),
798 f799 f
799 )800 )
800 self.assertFalse(path.exists(f))801 self.assertFalse(path.exists(f))
801 self.assertFalse(path.exists(d))802 self.assertFalse(path.exists(d))
802 self.assertEqual(803 self.assertEqual(
803 inst.temp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', create=True),804 inst.tmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', create=True),
804 f805 f
805 )806 )
806 self.assertFalse(path.exists(f))807 self.assertFalse(path.exists(f))
@@ -894,7 +895,113 @@
894 self.assertEqual(path.dirname(fp.name), imports)895 self.assertEqual(path.dirname(fp.name), imports)
895 self.assertTrue(fp.name.endswith('.mov'))896 self.assertTrue(fp.name.endswith('.mov'))
896897
897 def test_finalize_transfer(self):898 def test_tmp_move(self):
899 tmp = TempDir()
900 base = tmp.join('.dmedia')
901 dst_d = tmp.join('.dmedia', mov_hash[:2])
902 dst = tmp.join('.dmedia', mov_hash[:2], mov_hash[2:] + '.mov')
903 inst = self.klass(base)
904
905 # Test with wrong tmp_fp type
906 e = raises(TypeError, inst.tmp_move, 17, mov_hash, 'mov')
907 self.assertEqual(str(e), TYPE_ERROR % ('tmp_fp', file, int, 17))
908 self.assertFalse(path.exists(dst_d))
909 self.assertFalse(path.exists(dst))
910
911 # Test with tmp_fp opened in wrong mode
912 bad = tmp.touch('.dmedia', 'bad1.mov')
913 for mode in ('r', 'w', 'r+'):
914 fp = open(bad, mode)
915 e = raises(ValueError, inst.tmp_move, fp, mov_hash, 'mov')
916 self.assertEqual(
917 str(e),
918 "tmp_fp: mode must be 'rb', 'wb', or 'r+b'; got %r" % mode
919 )
920 self.assertFalse(path.exists(dst_d))
921 self.assertFalse(path.exists(dst))
922 self.assertFalse(fp.closed)
923
924 # Test when filename isn't contained in base (to ensure that
925 # FileStore.check_path() is used to validate tmp_fp.name)
926 bad = tmp.touch('.dmedia', '..', 'bad2.mov')
927 fp = open(bad, 'rb')
928 e = raises(FileStoreTraversal, inst.tmp_move, fp, mov_hash, 'mov')
929 self.assertEqual(e.pathname, bad)
930 self.assertEqual(e.abspath, tmp.join('bad2.mov'))
931 self.assertEqual(e.base, base)
932 self.assertFalse(path.exists(dst_d))
933 self.assertFalse(path.exists(dst))
934 self.assertFalse(fp.closed)
935
936 # Test that chash is validated with safe_b32()
937 good = tmp.write('yup', '.dmedia', 'imports', 'good.mov')
938 os.chmod(good, 0o660)
939 fp = open(good, 'rb')
940 b32 = 'NWBNVXVK5DQGIOW7MYR4K3KA'
941 e = raises(ValueError, inst.tmp_move, fp, b32, 'mov')
942 self.assertEqual(
943 str(e),
944 "len(b32) must be 32; got 24: 'NWBNVXVK5DQGIOW7MYR4K3KA'"
945 )
946 self.assertFalse(path.exists(dst_d))
947 self.assertFalse(path.exists(dst))
948 self.assertFalse(fp.closed)
949
950 # Test that ext is validate with safe_ext()
951 bad_ext = '/etc/ssh'
952 e = raises(ValueError, inst.tmp_move, fp, mov_hash, bad_ext)
953 self.assertEqual(
954 str(e),
955 'ext %r does not match pattern %r' % (bad_ext, EXT_PAT)
956 )
957 self.assertFalse(path.exists(dst_d))
958 self.assertFalse(path.exists(dst))
959 self.assertFalse(fp.closed)
960
961 # Test when tmp_fp is closed
962 fp.close()
963 e = raises(ValueError, inst.tmp_move, fp, mov_hash, 'mov')
964 self.assertEqual(
965 str(e),
966 'tmp_fp is closed, must be open: %r' % good
967 )
968 self.assertFalse(path.exists(dst_d))
969 self.assertFalse(path.exists(dst))
970 self.assertTrue(fp.closed)
971
972 # Test when it's all good
973 fp = open(good, 'rb')
974 self.assertEqual(stat.S_IMODE(os.stat(good).st_mode), 0o660)
975 self.assertEqual(inst.tmp_move(fp, mov_hash, 'mov'), dst)
976 self.assertFalse(path.exists(good))
977 self.assertTrue(path.isdir(dst_d))
978 self.assertTrue(path.isfile(dst))
979 self.assertEqual(stat.S_IMODE(os.stat(dst).st_mode), 0o444)
980 self.assertEqual(open(dst, 'rb').read(), 'yup')
981 self.assertTrue(fp.closed)
982
983 # Test when it's a duplicate
984 dup = tmp.write('wowza', '.dmedia', 'imports', 'dup')
985 os.chmod(dup, 0o660)
986 fp = open(dup, 'rb')
987 e = raises(DuplicateFile, inst.tmp_move, fp, mov_hash, 'mov')
988 self.assertEqual(e.chash, mov_hash)
989 self.assertEqual(e.src, dup)
990 self.assertEqual(e.dst, dst)
991 self.assertFalse(fp.closed)
992
993 # Make sure dup wasn't altered
994 self.assertTrue(path.isfile(dup))
995 self.assertEqual(stat.S_IMODE(os.stat(dup).st_mode), 0o660)
996 self.assertEqual(open(dup, 'rb').read(), 'wowza')
997
998 # Make sure dst wasn't altered
999 self.assertTrue(path.isdir(dst_d))
1000 self.assertTrue(path.isfile(dst))
1001 self.assertEqual(stat.S_IMODE(os.stat(dst).st_mode), 0o444)
1002 self.assertEqual(open(dst, 'rb').read(), 'yup')
1003
1004 def test_tmp_verify_move(self):
898 tmp = TempDir()1005 tmp = TempDir()
899 inst = self.klass(tmp.path)1006 inst = self.klass(tmp.path)
9001007
@@ -904,22 +1011,22 @@
904 dst = tmp.join(mov_hash[:2], mov_hash[2:] + '.mov')1011 dst = tmp.join(mov_hash[:2], mov_hash[2:] + '.mov')
9051012
906 # Test when transfers/ dir doesn't exist:1013 # Test when transfers/ dir doesn't exist:
907 e = raises(IOError, inst.finalize_transfer, mov_hash, 'mov')1014 e = raises(IOError, inst.tmp_verify_move, mov_hash, 'mov')
908 self.assertFalse(path.exists(src_d))1015 self.assertFalse(path.exists(src_d))
909 self.assertFalse(path.exists(dst_d))1016 self.assertFalse(path.exists(dst_d))
910 self.assertFalse(path.exists(dst))1017 self.assertFalse(path.exists(dst))
9111018
912 # Test when transfers/ exists but file does not:1019 # Test when transfers/ exists but file does not:
913 self.assertEqual(inst.temp(mov_hash, 'mov', create=True), src)1020 self.assertEqual(inst.tmp(mov_hash, 'mov', create=True), src)
914 self.assertTrue(path.isdir(src_d))1021 self.assertTrue(path.isdir(src_d))
915 e = raises(IOError, inst.finalize_transfer, mov_hash, 'mov')1022 e = raises(IOError, inst.tmp_verify_move, mov_hash, 'mov')
916 self.assertFalse(path.exists(src))1023 self.assertFalse(path.exists(src))
917 self.assertFalse(path.exists(dst_d))1024 self.assertFalse(path.exists(dst_d))
918 self.assertFalse(path.exists(dst))1025 self.assertFalse(path.exists(dst))
9191026
920 # Test when file has wrong content hash and wrong size:1027 # Test when file has wrong content hash and wrong size:
921 open(src, 'wb').write(open(sample_thm, 'rb').read())1028 open(src, 'wb').write(open(sample_thm, 'rb').read())
922 e = raises(IntegrityError, inst.finalize_transfer, mov_hash, 'mov')1029 e = raises(IntegrityError, inst.tmp_verify_move, mov_hash, 'mov')
923 self.assertEqual(e.got, thm_hash)1030 self.assertEqual(e.got, thm_hash)
924 self.assertEqual(e.expected, mov_hash)1031 self.assertEqual(e.expected, mov_hash)
925 self.assertEqual(e.filename, src)1032 self.assertEqual(e.filename, src)
@@ -942,7 +1049,7 @@
942 fp2.close()1049 fp2.close()
943 self.assertEqual(path.getsize(sample_mov), path.getsize(src))1050 self.assertEqual(path.getsize(sample_mov), path.getsize(src))
9441051
945 e = raises(IntegrityError, inst.finalize_transfer, mov_hash, 'mov')1052 e = raises(IntegrityError, inst.tmp_verify_move, mov_hash, 'mov')
946 self.assertEqual(e.got, 'UECTT7A7EIHZ2SGGBMMO5WTTSVU4SUWM')1053 self.assertEqual(e.got, 'UECTT7A7EIHZ2SGGBMMO5WTTSVU4SUWM')
947 self.assertEqual(e.expected, mov_hash)1054 self.assertEqual(e.expected, mov_hash)
948 self.assertEqual(e.filename, src)1055 self.assertEqual(e.filename, src)
@@ -959,7 +1066,7 @@
959 fp2.write(chunk)1066 fp2.write(chunk)
960 fp1.close()1067 fp1.close()
961 fp2.close()1068 fp2.close()
962 self.assertEqual(inst.finalize_transfer(mov_hash, 'mov'), dst)1069 self.assertEqual(inst.tmp_verify_move(mov_hash, 'mov'), dst)
963 self.assertTrue(path.isdir(src_d))1070 self.assertTrue(path.isdir(src_d))
964 self.assertFalse(path.exists(src))1071 self.assertFalse(path.exists(src))
965 self.assertTrue(path.isdir(dst_d))1072 self.assertTrue(path.isdir(dst_d))

Subscribers

People subscribed via source and target branches