( )⚙ D4865 testing: add file storage integration for bad hashes and censoring

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4865

testing: add file storage integration for bad hashes and censoring
ClosedPublic

Authored by indygreg on Oct 3 2018, 2:13 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGcdf61ab1f54c: testing: add file storage integration for bad hashes and censoring
rHGf85bf10ab511: testing: add file storage integration for bad hashes and censoring
Summary

In order to implement these tests, we need a backdoor to write data
into storage backends while bypassing normal checks. We invent a
callable to do that.

As part of writing the tests, I found a bug with censorrevision()
pretty quickly! After calling censorrevision(), attempting to
access revision data for an affected node raises a cryptic error
related to malformed compression. This appears to be due to the
revlog not adjusting delta chains as part of censoring.

I also found a bug with regards to hash verification and revision
fulltext caching. Essentially, we cache the fulltext before hash
verification. If we look up the fulltext after a failed hash
verification, we don't get a hash verification exception. Furthermore,
the behavior of revision(raw=True) can be inconsistent depending on
the order of operations.

I'll be fixing both these bugs in subsequent commits.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Oct 3 2018, 2:13 PM
Closed by commit rHGf85bf10ab511: testing: add file storage integration for bad hashes and censoring (authored by indygreg). · Explain WhyOct 4 2018, 2:21 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/testing/storage.py (206 lines)
Mtests/test-storage.py (33 lines)

Diff 11646

mercurial/testing/storage.py

self.assertEqual(f.read(node1), fulltext1) self.assertEqual(f.read(node1), fulltext1)
self.assertFalse(f.cmp(node0, fulltext0)) self.assertFalse(f.cmp(node0, fulltext0))
self.assertTrue(f.cmp(node0, stored0)) self.assertTrue(f.cmp(node0, stored0))
self.assertFalse(f.cmp(node1, fulltext1)) self.assertFalse(f.cmp(node1, fulltext1))
self.assertTrue(f.cmp(node1, stored0)) self.assertTrue(f.cmp(node1, stored0))
def testbadnoderead(self):
f = self._makefilefn()
fulltext0 = b'foo\n' * 30
fulltext1 = fulltext0 + b'bar\n'
with self._maketransactionfn() as tr:
node0 = f.add(fulltext0, None, tr, 0, nullid, nullid)
node1 = b'\xaa' * 20
self._addrawrevisionfn(f, tr, node1, node0, nullid, 1,
rawtext=fulltext1)
self.assertEqual(len(f), 2)
self.assertEqual(f.parents(node1), (node0, nullid))
# revision() raises since it performs hash verification.
with self.assertRaises(error.StorageError):
f.revision(node1)
# revision(raw=True) still verifies hashes.
# TODO this is buggy because of cache interaction.
self.assertEqual(f.revision(node1, raw=True), fulltext1)
# read() behaves like revision().
# TODO this is buggy because of cache interaction.
f.read(node1)
# We can't test renamed() here because some backends may not require
# reading/validating the fulltext to return rename metadata.
def testbadnoderevisionraw(self):
# Like above except we test revision(raw=True) first to isolate
# revision caching behavior.
f = self._makefilefn()
fulltext0 = b'foo\n' * 30
fulltext1 = fulltext0 + b'bar\n'
with self._maketransactionfn() as tr:
node0 = f.add(fulltext0, None, tr, 0, nullid, nullid)
node1 = b'\xaa' * 20
self._addrawrevisionfn(f, tr, node1, node0, nullid, 1,
rawtext=fulltext1)
with self.assertRaises(error.StorageError):
f.revision(node1, raw=True)
with self.assertRaises(error.StorageError):
f.revision(node1, raw=True)
def testbadnoderevisionraw(self):
# Like above except we test read() first to isolate revision caching
# behavior.
f = self._makefilefn()
fulltext0 = b'foo\n' * 30
fulltext1 = fulltext0 + b'bar\n'
with self._maketransactionfn() as tr:
node0 = f.add(fulltext0, None, tr, 0, nullid, nullid)
node1 = b'\xaa' * 20
self._addrawrevisionfn(f, tr, node1, node0, nullid, 1,
rawtext=fulltext1)
with self.assertRaises(error.StorageError):
f.read(node1)
# TODO this should raise error.StorageError.
f.read(node1)
def testbadnodedelta(self):
f = self._makefilefn()
fulltext0 = b'foo\n' * 31
fulltext1 = fulltext0 + b'bar\n'
fulltext2 = fulltext1 + b'baz\n'
with self._maketransactionfn() as tr:
node0 = f.add(fulltext0, None, tr, 0, nullid, nullid)
node1 = b'\xaa' * 20
self._addrawrevisionfn(f, tr, node1, node0, nullid, 1,
rawtext=fulltext1)
with self.assertRaises(error.StorageError):
f.read(node1)
diff = mdiff.textdiff(fulltext1, fulltext2)
node2 = storageutil.hashrevisionsha1(fulltext2, node1, nullid)
deltas = [(node2, node1, nullid, b'\x01' * 20, node1, diff, 0)]
# This /might/ fail on some backends.
with self._maketransactionfn() as tr:
f.addgroup(deltas, lambda x: 0, tr)
self.assertEqual(len(f), 3)
# Assuming a delta is stored, we shouldn't need to validate node1 in
# order to retrieve node2.
self.assertEqual(f.read(node2), fulltext2)
def testcensored(self): def testcensored(self):
f = self._makefilefn() f = self._makefilefn()
stored1 = storageutil.packmeta({ stored1 = storageutil.packmeta({
b'censored': b'tombstone', b'censored': b'tombstone',
}, b'') }, b'')
# TODO tests are incomplete because we need the node to be
# different due to presence of censor metadata. But we can't
# do this with addrevision().
with self._maketransactionfn() as tr: with self._maketransactionfn() as tr:
node0 = f.add(b'foo', None, tr, 0, nullid, nullid) node0 = f.add(b'foo', None, tr, 0, nullid, nullid)
f.addrevision(stored1, tr, 1, node0, nullid,
flags=repository.REVISION_FLAG_CENSORED) # The node value doesn't matter since we can't verify it.
node1 = b'\xbb' * 20
self._addrawrevisionfn(f, tr, node1, node0, nullid, 1, stored1,
censored=True)
self.assertTrue(f.iscensored(1)) self.assertTrue(f.iscensored(1))
self.assertEqual(f.revision(1), stored1) with self.assertRaises(error.CensoredNodeError):
f.revision(1)
self.assertEqual(f.revision(1, raw=True), stored1) self.assertEqual(f.revision(1, raw=True), stored1)
self.assertEqual(f.read(1), b'') with self.assertRaises(error.CensoredNodeError):
f.read(1)
def testcensoredrawrevision(self):
# Like above, except we do the revision(raw=True) request first to
# isolate revision caching behavior.
f = self._makefilefn()
stored1 = storageutil.packmeta({
b'censored': b'tombstone',
}, b'')
with self._maketransactionfn() as tr:
node0 = f.add(b'foo', None, tr, 0, nullid, nullid)
# The node value doesn't matter since we can't verify it.
node1 = b'\xbb' * 20
self._addrawrevisionfn(f, tr, node1, node0, nullid, 1, stored1,
censored=True)
with self.assertRaises(error.CensoredNodeError):
f.revision(1, raw=True)
class ifilemutationtests(basetestcase): class ifilemutationtests(basetestcase):
"""Generic tests for the ifilemutation interface. """Generic tests for the ifilemutation interface.
All file storage backends that support writing should conform to this All file storage backends that support writing should conform to this
interface. interface.
Use ``makeifilemutationtests()`` to create an instance of this type. Use ``makeifilemutationtests()`` to create an instance of this type.
self.assertEqual(list(f.revs()), [0, 1, 2]) self.assertEqual(list(f.revs()), [0, 1, 2])
self.assertEqual(f.rev(nodes[0]), 0) self.assertEqual(f.rev(nodes[0]), 0)
self.assertEqual(f.rev(nodes[1]), 1) self.assertEqual(f.rev(nodes[1]), 1)
self.assertEqual(f.rev(nodes[2]), 2) self.assertEqual(f.rev(nodes[2]), 2)
self.assertEqual(f.node(0), nodes[0]) self.assertEqual(f.node(0), nodes[0])
self.assertEqual(f.node(1), nodes[1]) self.assertEqual(f.node(1), nodes[1])
self.assertEqual(f.node(2), nodes[2]) self.assertEqual(f.node(2), nodes[2])
def testdeltaagainstcensored(self):
# Attempt to apply a delta made against a censored revision.
f = self._makefilefn()
stored1 = storageutil.packmeta({
b'censored': b'tombstone',
}, b'')
with self._maketransactionfn() as tr:
node0 = f.add(b'foo\n' * 30, None, tr, 0, nullid, nullid)
# The node value doesn't matter since we can't verify it.
node1 = b'\xbb' * 20
self._addrawrevisionfn(f, tr, node1, node0, nullid, 1, stored1,
censored=True)
delta = mdiff.textdiff(b'bar\n' * 30, (b'bar\n' * 30) + b'baz\n')
deltas = [(b'\xcc' * 20, node1, nullid, b'\x01' * 20, node1, delta, 0)]
with self._maketransactionfn() as tr:
with self.assertRaises(error.CensoredBaseError):
f.addgroup(deltas, lambda x: 0, tr)
def testcensorrevisionbasic(self):
f = self._makefilefn()
with self._maketransactionfn() as tr:
node0 = f.add(b'foo\n' * 30, None, tr, 0, nullid, nullid)
node1 = f.add(b'foo\n' * 31, None, tr, 1, node0, nullid)
node2 = f.add(b'foo\n' * 32, None, tr, 2, node1, nullid)
with self._maketransactionfn() as tr:
f.censorrevision(tr, node1)
self.assertEqual(len(f), 3)
self.assertEqual(list(f.revs()), [0, 1, 2])
self.assertEqual(f.read(node0), b'foo\n' * 30)
# TODO revlog can't resolve revision after censor. Probably due to a
# cache on the revlog instance.
with self.assertRaises(error.StorageError):
self.assertEqual(f.read(node2), b'foo\n' * 32)
# TODO should raise CensoredNodeError, but fallout from above prevents.
with self.assertRaises(error.StorageError):
f.read(node1)
def testgetstrippointnoparents(self): def testgetstrippointnoparents(self):
# N revisions where none have parents. # N revisions where none have parents.
f = self._makefilefn() f = self._makefilefn()
with self._maketransactionfn() as tr: with self._maketransactionfn() as tr:
for rev in range(10): for rev in range(10):
f.add(b'%d' % rev, None, tr, rev, nullid, nullid) f.add(b'%d' % rev, None, tr, rev, nullid, nullid)
with self._maketransactionfn() as tr: with self._maketransactionfn() as tr:
f.strip(3, tr) f.strip(3, tr)
self.assertEqual(len(f), 1) self.assertEqual(len(f), 1)
with self.assertRaises(error.LookupError): with self.assertRaises(error.LookupError):
f.rev(node1) f.rev(node1)
def makeifileindextests(makefilefn, maketransactionfn): def makeifileindextests(makefilefn, maketransactionfn, addrawrevisionfn):
"""Create a unittest.TestCase class suitable for testing file storage. """Create a unittest.TestCase class suitable for testing file storage.
``makefilefn`` is a callable which receives the test case as an ``makefilefn`` is a callable which receives the test case as an
argument and returns an object implementing the ``ifilestorage`` interface. argument and returns an object implementing the ``ifilestorage`` interface.
``maketransactionfn`` is a callable which receives the test case as an ``maketransactionfn`` is a callable which receives the test case as an
argument and returns a transaction object. argument and returns a transaction object.
``addrawrevisionfn`` is a callable which receives arguments describing a
low-level revision to add. This callable allows the insertion of
potentially bad data into the store in order to facilitate testing.
Returns a type that is a ``unittest.TestCase`` that can be used for Returns a type that is a ``unittest.TestCase`` that can be used for
testing the object implementing the file storage interface. Simply testing the object implementing the file storage interface. Simply
assign the returned value to a module-level attribute and a test loader assign the returned value to a module-level attribute and a test loader
should find and run it automatically. should find and run it automatically.
""" """
d = { d = {
r'_makefilefn': makefilefn, r'_makefilefn': makefilefn,
r'_maketransactionfn': maketransactionfn, r'_maketransactionfn': maketransactionfn,
r'_addrawrevisionfn': addrawrevisionfn,
} }
return type(r'ifileindextests', (ifileindextests,), d) return type(r'ifileindextests', (ifileindextests,), d)
def makeifiledatatests(makefilefn, maketransactionfn): def makeifiledatatests(makefilefn, maketransactionfn, addrawrevisionfn):
d = { d = {
r'_makefilefn': makefilefn, r'_makefilefn': makefilefn,
r'_maketransactionfn': maketransactionfn, r'_maketransactionfn': maketransactionfn,
r'_addrawrevisionfn': addrawrevisionfn,
} }
return type(r'ifiledatatests', (ifiledatatests,), d) return type(r'ifiledatatests', (ifiledatatests,), d)
def makeifilemutationtests(makefilefn, maketransactionfn): def makeifilemutationtests(makefilefn, maketransactionfn, addrawrevisionfn):
d = { d = {
r'_makefilefn': makefilefn, r'_makefilefn': makefilefn,
r'_maketransactionfn': maketransactionfn, r'_maketransactionfn': maketransactionfn,
r'_addrawrevisionfn': addrawrevisionfn,
} }
return type(r'ifilemutationtests', (ifilemutationtests,), d) return type(r'ifilemutationtests', (ifilemutationtests,), d)

tests/test-storage.py

# This test verifies the conformance of various classes to various # This test verifies the conformance of various classes to various
# storage interfaces. # storage interfaces.
from __future__ import absolute_import from __future__ import absolute_import
import silenttestrunner import silenttestrunner
from mercurial import ( from mercurial import (
error,
filelog, filelog,
revlog,
transaction, transaction,
ui as uimod, ui as uimod,
vfs as vfsmod, vfs as vfsmod,
) )
from mercurial.testing import ( from mercurial.testing import (
storage as storagetesting, storage as storagetesting,
) )
return fl return fl
def maketransaction(self): def maketransaction(self):
vfsmap = {'plain': STATE['vfs']} vfsmap = {'plain': STATE['vfs']}
return transaction.transaction(STATE['ui'].warn, STATE['vfs'], vfsmap, return transaction.transaction(STATE['ui'].warn, STATE['vfs'], vfsmap,
b'journal', b'undo') b'journal', b'undo')
def addrawrevision(self, fl, tr, node, p1, p2, linkrev, rawtext=None,
delta=None, censored=False, ellipsis=False, extstored=False):
flags = 0
if censored:
flags |= revlog.REVIDX_ISCENSORED
if ellipsis:
flags |= revlog.REVIDX_ELLIPSIS
if extstored:
flags |= revlog.REVIDX_EXTSTORED
if rawtext is not None:
fl._revlog.addrawrevision(rawtext, tr, linkrev, p1, p2, node, flags)
elif delta is not None:
raise error.Abort('support for storing raw deltas not yet supported')
else:
raise error.Abort('must supply rawtext or delta arguments')
# We may insert bad data. Clear caches to prevent e.g. cache hits to
# bypass hash verification.
fl._revlog.clearcaches()
# Assigning module-level attributes that inherit from unittest.TestCase # Assigning module-level attributes that inherit from unittest.TestCase
# is all that is needed to register tests. # is all that is needed to register tests.
filelogindextests = storagetesting.makeifileindextests(makefilefn, filelogindextests = storagetesting.makeifileindextests(makefilefn,
maketransaction) maketransaction,
addrawrevision)
filelogdatatests = storagetesting.makeifiledatatests(makefilefn, filelogdatatests = storagetesting.makeifiledatatests(makefilefn,
maketransaction) maketransaction,
addrawrevision)
filelogmutationtests = storagetesting.makeifilemutationtests(makefilefn, filelogmutationtests = storagetesting.makeifilemutationtests(makefilefn,
maketransaction) maketransaction,
addrawrevision)
if __name__ == '__main__': if __name__ == '__main__':
silenttestrunner.main(__name__) silenttestrunner.main(__name__)