From patchwork Mon Jul 6 01:46:16 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: D8679: manifest: tigher manifest parsing and flag use From: phabricator X-Patchwork-Id: 46626 Message-Id: To: Phabricator Cc: mercurial-devel@mercurial-scm.org Date: Mon, 6 Jul 2020 01:46:16 +0000 joerg.sonnenberger created this revision. Herald added a reviewer: hg-reviewers. Herald added a subscriber: mercurial-patches. REVISION SUMMARY In the manifest line, flags are put directly after the hash, so the parser has been guessing the presence of flags based on the length of the hash. Replace this assumption by an enumeration of the valid flags and removing them from the hash first as they are distinct input values. Consistently handle the expected 256bit length of the SHA1-replacement in the pure Python prser as well. Check that setting flags will use one of the blessed values. REPOSITORY rHG Mercurial BRANCH default REVISION DETAIL https://phab.mercurial-scm.org/D8679 AFFECTED FILES mercurial/cext/manifest.c mercurial/manifest.py CHANGE DETAILS To: joerg.sonnenberger, #hg-reviewers Cc: mercurial-patches, mercurial-devel diff --git a/mercurial/manifest.py b/mercurial/manifest.py --- a/mercurial/manifest.py +++ b/mercurial/manifest.py @@ -121,8 +121,20 @@ self.pos += 1 return data zeropos = data.find(b'\x00', pos) - hashval = unhexlify(data, self.lm.extrainfo[self.pos], zeropos + 1, 40) - flags = self.lm._getflags(data, self.pos, zeropos) + nlpos = data.find(b'\n', pos) + if zeropos == -1 or nlpos == -1 or nlpos < zeropos: + raise error.StorageError(b'Invalid manifest line') + flags = data[nlpos - 1 : nlpos] + if flags in _manifestflags: + hlen = nlpos - zeropos - 2 + else: + hlen = nlpos - zeropos - 1 + flags = b'' + if hlen not in (40, 64): + raise error.StorageError(b'Invalid manifest line') + hashval = unhexlify( + data, self.lm.extrainfo[self.pos], zeropos + 1, hlen + ) self.pos += 1 return (data[pos:zeropos], hashval, flags) @@ -140,6 +152,9 @@ return (a > b) - (a < b) +_manifestflags = {b'', b'l', b't', b'x'} + + class _lazymanifest(object): """A pure python manifest backed by a byte string. It is supplimented with internal lists as it is modified, until it is compacted back to a pure byte @@ -251,15 +266,6 @@ def __contains__(self, key): return self.bsearch(key) != -1 - def _getflags(self, data, needle, pos): - start = pos + 41 - end = data.find(b"\n", start) - if end == -1: - end = len(data) - 1 - if start == end: - return b'' - return self.data[start:end] - def __getitem__(self, key): if not isinstance(key, bytes): raise TypeError(b"getitem: manifest keys must be a bytes.") @@ -273,13 +279,17 @@ nlpos = data.find(b'\n', zeropos) assert 0 <= needle <= len(self.positions) assert len(self.extrainfo) == len(self.positions) + if zeropos == -1 or nlpos == -1 or nlpos < zeropos: + raise error.StorageError(b'Invalid manifest line') hlen = nlpos - zeropos - 1 - # Hashes sometimes have an extra byte tucked on the end, so - # detect that. - if hlen % 2: + flags = data[nlpos - 1 : nlpos] + if flags in _manifestflags: hlen -= 1 + else: + flags = b'' + if hlen not in (40, 64): + raise error.StorageError(b'Invalid manifest line') hashval = unhexlify(data, self.extrainfo[needle], zeropos + 1, hlen) - flags = self._getflags(data, needle, zeropos) return (hashval, flags) def __delitem__(self, key): @@ -609,6 +619,8 @@ return self._lm.diff(m2._lm, clean) def setflag(self, key, flag): + if flag not in _manifestflags: + raise TypeError(b"Invalid manifest flag set.") self._lm[key] = self[key], flag def get(self, key, default=None): @@ -1066,6 +1078,8 @@ def setflag(self, f, flags): """Set the flags (symlink, executable) for path f.""" + if flags not in _manifestflags: + raise TypeError(b"Invalid manifest flag set.") self._load() dir, subpath = _splittopdir(f) if dir: diff --git a/mercurial/cext/manifest.c b/mercurial/cext/manifest.c --- a/mercurial/cext/manifest.c +++ b/mercurial/cext/manifest.c @@ -49,23 +49,37 @@ } /* get the node value of a single line */ -static PyObject *nodeof(line *l) +static PyObject *nodeof(line *l, char *flag) { char *s = l->start; Py_ssize_t llen = pathlen(l); Py_ssize_t hlen = l->len - llen - 2; - Py_ssize_t hlen_raw = 20; + Py_ssize_t hlen_raw; PyObject *hash; if (llen + 1 + 40 + 1 > l->len) { /* path '\0' hash '\n' */ PyErr_SetString(PyExc_ValueError, "manifest line too short"); return NULL; } + /* Detect flags after the hash first. */ + switch (s[llen + hlen]) { + case 'l': + case 't': + case 'x': + if (flag) + *flag = s[llen + hlen]; + --hlen; + break; + default: + if (flag) + *flag = '\0'; + break; + } + switch (hlen) { case 40: /* sha1 */ - case 41: /* sha1 with cruft for a merge */ + hlen_raw = 20; break; case 64: /* new hash */ - case 65: /* new hash with cruft for a merge */ hlen_raw = 32; break; default: @@ -89,9 +103,8 @@ /* get the node hash and flags of a line as a tuple */ static PyObject *hashflags(line *l) { - char *s = l->start; - Py_ssize_t plen = pathlen(l); - PyObject *hash = nodeof(l); + char flag; + PyObject *hash = nodeof(l, &flag); ssize_t hlen; Py_ssize_t hplen, flen; PyObject *flags; @@ -99,14 +112,7 @@ if (!hash) return NULL; - /* hash is either 20 or 21 bytes for an old hash, so we use a - ternary here to get the "real" hexlified sha length. */ - hlen = PyBytes_GET_SIZE(hash) < 22 ? 40 : 64; - /* 1 for null byte, 1 for newline */ - hplen = plen + hlen + 2; - flen = l->len - hplen; - - flags = PyBytes_FromStringAndSize(s + hplen - 1, flen); + flags = PyBytes_FromStringAndSize(&flag, flag ? 1 : 0); if (!flags) { Py_DECREF(hash); return NULL; @@ -299,7 +305,7 @@ } pl = pathlen(l); path = PyBytes_FromStringAndSize(l->start, pl); - hash = nodeof(l); + hash = nodeof(l, NULL); if (!path || !hash) { goto done; }