# HG changeset patch # User Gregory Szorc # Date 1538526874 25200 # Node ID 324b4b10351ef76d58014329d349ff59cdd3ff24 # Parent 0a4625ffd6c0fa3a078c6875078039331a671139 revlog: rewrite censoring logic I was able to corrupt a revlog relatively easily with the existing censoring code. The underlying problem is that the existing code doesn't fully take delta chains into account. When copying revisions that occur after the censored revision, the delta base can refer to a censored revision. Then at read time, things blow up due to the revision data not being a compressed delta. This commit rewrites the revlog censoring code to take a higher-level approach. We now create a new revlog instance pointing at temp files. We iterate through each revision in the source revlog and insert those revisions into the new revlog, replacing the censored revision's data along the way. The new implementation isn't as efficient as the old one. This is because it will fully engage delta computation on insertion. But I don't think it matters. The new implementation is a bit hacky because it attempts to reload the revlog instance with a new revlog index/data file. This is fragile. But this is needed because the index (which could be backed by C) would have a cached copy of the old, possibly changed data and that could lead to problems accessing index or revision data later. One benefit of the new approach is that we integrate with the transaction. The old revlog is backed up and if the transaction is rolled back, the original revlog is restored. As part of this, we had to teach the transaction about the store vfs. I'm not super keen about this. But this was the easiest way to hook things up to the transaction. We /could/ just ignore the transaction like we were doing before. But any file mutation should be governed by transaction semantics, including undo during rollback. Differential Revision: https://phab.mercurial-scm.org/D4869 diff -r 0a4625ffd6c0 -r 324b4b10351e mercurial/filelog.py --- a/mercurial/filelog.py Tue Oct 02 17:28:54 2018 -0700 +++ b/mercurial/filelog.py Tue Oct 02 17:34:34 2018 -0700 @@ -101,7 +101,7 @@ return self._revlog.strip(minlink, transaction) def censorrevision(self, tr, node, tombstone=b''): - return self._revlog.censorrevision(node, tombstone=tombstone) + return self._revlog.censorrevision(tr, node, tombstone=tombstone) def files(self): return self._revlog.files() diff -r 0a4625ffd6c0 -r 324b4b10351e mercurial/localrepo.py --- a/mercurial/localrepo.py Tue Oct 02 17:28:54 2018 -0700 +++ b/mercurial/localrepo.py Tue Oct 02 17:34:34 2018 -0700 @@ -1682,7 +1682,7 @@ rp = report else: rp = self.ui.warn - vfsmap = {'plain': self.vfs} # root of .hg/ + vfsmap = {'plain': self.vfs, 'store': self.svfs} # root of .hg/ # we must avoid cyclic reference between repo and transaction. reporef = weakref.ref(self) # Code to track tag movement diff -r 0a4625ffd6c0 -r 324b4b10351e mercurial/revlog.py --- a/mercurial/revlog.py Tue Oct 02 17:28:54 2018 -0700 +++ b/mercurial/revlog.py Tue Oct 02 17:34:34 2018 -0700 @@ -2341,94 +2341,69 @@ destrevlog._lazydeltabase = oldlazydeltabase destrevlog._deltabothparents = oldamd - def censorrevision(self, node, tombstone=b''): + def censorrevision(self, tr, censornode, tombstone=b''): if (self.version & 0xFFFF) == REVLOGV0: raise error.RevlogError(_('cannot censor with version %d revlogs') % self.version) - rev = self.rev(node) + censorrev = self.rev(censornode) tombstone = storageutil.packmeta({b'censored': tombstone}, b'') - if len(tombstone) > self.rawsize(rev): + if len(tombstone) > self.rawsize(censorrev): raise error.Abort(_('censor tombstone must be no longer than ' 'censored data')) - # Using two files instead of one makes it easy to rewrite entry-by-entry - idxread = self.opener(self.indexfile, 'r') - idxwrite = self.opener(self.indexfile, 'wb', atomictemp=True) - if self.version & FLAG_INLINE_DATA: - dataread, datawrite = idxread, idxwrite - else: - dataread = self.opener(self.datafile, 'r') - datawrite = self.opener(self.datafile, 'wb', atomictemp=True) + # Rewriting the revlog in place is hard. Our strategy for censoring is + # to create a new revlog, copy all revisions to it, then replace the + # revlogs on transaction close. - # Copy all revlog data up to the entry to be censored. - offset = self.start(rev) - - for chunk in util.filechunkiter(idxread, limit=rev * self._io.size): - idxwrite.write(chunk) - for chunk in util.filechunkiter(dataread, limit=offset): - datawrite.write(chunk) + newindexfile = self.indexfile + b'.tmpcensored' + newdatafile = self.datafile + b'.tmpcensored' - def rewriteindex(r, newoffs, newdata=None): - """Rewrite the index entry with a new data offset and new data. + # This is a bit dangerous. We could easily have a mismatch of state. + newrl = revlog(self.opener, newindexfile, newdatafile, + censorable=True) + newrl.version = self.version + newrl._generaldelta = self._generaldelta + newrl._io = self._io - The newdata argument, if given, is a tuple of three positive - integers: (new compressed, new uncompressed, added flag bits). - """ - offlags, comp, uncomp, base, link, p1, p2, nodeid = self.index[r] - flags = gettype(offlags) - if newdata: - comp, uncomp, nflags = newdata - flags |= nflags - offlags = offset_type(newoffs, flags) - e = (offlags, comp, uncomp, r, link, p1, p2, nodeid) - idxwrite.write(self._io.packentry(e, None, self.version, r)) - idxread.seek(self._io.size, 1) + for rev in self.revs(): + node = self.node(rev) + p1, p2 = self.parents(node) - def rewrite(r, offs, data, nflags=REVIDX_DEFAULT_FLAGS): - """Write the given fulltext with the given data offset. + if rev == censorrev: + newrl.addrawrevision(tombstone, tr, self.linkrev(censorrev), + p1, p2, censornode, REVIDX_ISCENSORED) - Returns: - The integer number of data bytes written, for tracking data - offsets. - """ - flag, compdata = self.compress(data) - newcomp = len(flag) + len(compdata) - rewriteindex(r, offs, (newcomp, len(data), nflags)) - datawrite.write(flag) - datawrite.write(compdata) - dataread.seek(self.length(r), 1) - return newcomp - - # Rewrite censored entry with (padded) tombstone data. - pad = ' ' * (self.rawsize(rev) - len(tombstone)) - offset += rewrite(rev, offset, tombstone + pad, REVIDX_ISCENSORED) + if newrl.deltaparent(rev) != nullrev: + raise error.Abort(_('censored revision stored as delta; ' + 'cannot censor'), + hint=_('censoring of revlogs is not ' + 'fully implemented; please report ' + 'this bug')) + continue - # Rewrite all following filelog revisions fixing up offsets and deltas. - for srev in pycompat.xrange(rev + 1, len(self)): - if rev in self.parentrevs(srev): - # Immediate children of censored node must be re-added as - # fulltext. - try: - revdata = self.revision(srev) - except error.CensoredNodeError as e: - revdata = e.tombstone - dlen = rewrite(srev, offset, revdata) + if self.iscensored(rev): + if self.deltaparent(rev) != nullrev: + raise error.Abort(_('cannot censor due to censored ' + 'revision having delta stored')) + rawtext = self._chunk(rev) else: - # Copy any other revision data verbatim after fixing up the - # offset. - rewriteindex(srev, offset) - dlen = self.length(srev) - for chunk in util.filechunkiter(dataread, limit=dlen): - datawrite.write(chunk) - offset += dlen + rawtext = self.revision(rev, raw=True) + + newrl.addrawrevision(rawtext, tr, self.linkrev(rev), p1, p2, node, + self.flags(rev)) - idxread.close() - idxwrite.close() - if dataread is not idxread: - dataread.close() - datawrite.close() + tr.addbackup(self.indexfile, location='store') + if not self._inline: + tr.addbackup(self.datafile, location='store') + + self.opener.rename(newrl.indexfile, self.indexfile) + if not self._inline: + self.opener.rename(newrl.datafile, self.datafile) + + self.clearcaches() + self._loadindex(self.version, None) def verifyintegrity(self, state): """Verifies the integrity of the revlog. diff -r 0a4625ffd6c0 -r 324b4b10351e mercurial/testing/storage.py --- a/mercurial/testing/storage.py Tue Oct 02 17:28:54 2018 -0700 +++ b/mercurial/testing/storage.py Tue Oct 02 17:34:34 2018 -0700 @@ -1175,14 +1175,9 @@ self.assertEqual(list(f.revs()), [0, 1, 2]) self.assertEqual(f.read(node0), b'foo\n' * 30) + self.assertEqual(f.read(node2), b'foo\n' * 32) - # 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): + with self.assertRaises(error.CensoredNodeError): f.read(node1) def testgetstrippointnoparents(self): diff -r 0a4625ffd6c0 -r 324b4b10351e tests/test-storage.py --- a/tests/test-storage.py Tue Oct 02 17:28:54 2018 -0700 +++ b/tests/test-storage.py Tue Oct 02 17:34:34 2018 -0700 @@ -30,7 +30,7 @@ return fl def maketransaction(self): - vfsmap = {'plain': STATE['vfs']} + vfsmap = {'plain': STATE['vfs'], 'store': STATE['vfs']} return transaction.transaction(STATE['ui'].warn, STATE['vfs'], vfsmap, b'journal', b'undo')