[PATCH] revlog: ensure that flags do not overflow 2 bytes

[Cotizo Sima]

Gregory, I agree, but in this particular case I chose the “deepest” call which insert flags into the revlog such that I cover all the possible call paths. Although to be fair, another good alternative would be revlog._addrevision. I’m happy to move the operation there, if you guys find it more appropriate.

From: Gregory Szorc <gregory.szorc at gmail.com>
Date: Friday, November 25, 2016 at 5:27 PM
To: Cotizo Sima <cotizo at fb.com>
Cc: mercurial-devel <mercurial-devel at mercurial-scm.org>
Subject: Re: [PATCH] revlog: ensure that flags do not overflow 2 bytes

On Fri, Nov 25, 2016 at 5:24 AM, Cotizo Sima <cotizo at fb.com<mailto:cotizo at fb.com>> wrote:
# HG changeset patch
# User Cotizo Sima <cotizo at fb.com<mailto:cotizo at fb.com>>
# Date 1480079985 28800
#      Fri Nov 25 05:19:45 2016 -0800
# Node ID 4b311b730614941db6409f560ab9451bec74be75
# Parent  a3163433647108b7bec8fc45896db1c20b18ab21
revlog: ensure that flags do not overflow 2 bytes

This patch adds a line that ensures we are not setting by mistake a set of flags
overlfowing the 2 bytes they are allocated. Given the way the data is packed in
the revlog header, overflowing 2 bytes will result in setting a wrong offset.

diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -72,6 +72,7 @@
     return int(q & 0xFFFF)

 def offset_type(offset, type):
+    type &= 0xFFFF
     return long(long(offset) << 16 | type)

 _nullhash = hashlib.sha1(nullid)

I'm a believer in failing fast. If this new code comes into play, we've already lost to a bug. I think instead we should raise ValueError if type > 65535.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-devel/attachments/20161125/7066c3e2/attachment-0002.html>