Hi Filippo,

On Sat, Aug 01, 2009 at 09:46:59AM +0100, Filippo Giunchedi wrote:
> On Wed, Jul 29, 2009 at 03:44:53PM +0200, John Wright wrote:
> > Only now I actually understand what the XXX comment was asking...
> > Really, we want to make sure that values don't end in a newline, or
> > we'll effectively be making a new stanza when we add the newline to it
> > here.
> > 
> > I wonder if it would actually make more sense to have a value-sanitizer
> > method, which basically returns value.strip() after first making sure
> > there are no empty lines in value.
> 
> is it going to be reused elsewhere? Perhaps clarifying the comment would be
> enough, adding a function call might slow down dump() at least AFAICT

How about just validating in Deb822.__setitem__?  This may slightly slow
down parsing without apt_pkg, but it won't have any effect when using
apt_pkg unless you change a field value after the fact.  It won't affect
dump() at all.

Patch attached for review.

-- 
John Wright <[email protected]>
>From bf321ad6fbdd0541d3b972ab8ec090f4d54d50ef Mon Sep 17 00:00:00 2001
From: John Wright <[email protected]>
Date: Wed, 29 Jul 2009 15:17:27 +0200
Subject: [PATCH] deb822: Validate input in Deb822.__setitem__

Try to catch input that would result in multiple stanzas or unparseable
data when dumping the Deb822 object.

This patch also clarifies a comment related to this, and removes an XXX
comment that it addreses.
---
 debian_bundle/deb822.py |   35 ++++++++++++++++++++++++++++++++---
 1 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/debian_bundle/deb822.py b/debian_bundle/deb822.py
index dd1d201..ca35668 100644
--- a/debian_bundle/deb822.py
+++ b/debian_bundle/deb822.py
@@ -358,10 +358,12 @@ class Deb822(Deb822Dict):
         else:
             return_string = False
         for key, value in self.iteritems():
+            # We want one space between the "Field:" and value, unless
+            # value starts with a newline (i.e. the value itself started on
+            # the line *after* the field name in the control file), or is
+            # empty.  In that case, we avoid trailing whitespace by by not
+            # including a space after the colon.
             if not value or value[0] == '\n':
-                # Avoid trailing whitespace after "Field:" if it's on its own
-                # line or the value is empty
-                # XXX Uh, really print value if value == '\n'?
                 fd.write('%s:%s\n' % (key, value))
             else:
                 fd.write('%s: %s\n' % (key, value))
@@ -538,6 +540,33 @@ class Deb822(Deb822Dict):
 
         return self.gpg_info
 
+    def validate_input(self, key, value):
+        """Raise ValueError if value is not a valid value for key
+
+        Subclasses that do interesting things for different keys may wish to
+        override this method.
+        """
+
+        strvalue = str(value)
+
+        # The value cannot end in a newline (if it did, dumping the object
+        # would result in multiple stanzas)
+        if strvalue.endswith('\n'):
+            raise ValueError("value must not end in '\\n'")
+
+        # Make sure there are no blank lines (actually, the first one is
+        # allowed to be blank, but no others), and each subsequent line starts
+        # with whitespace
+        for line in strvalue.splitlines()[1:]:
+            if not line:
+                raise ValueError("value must not have blank lines")
+            if not line[0] in string.whitespace:
+                raise ValueError("each line must start with whitespace")
+
+    def __setitem__(self, key, value):
+        self.validate_input(key, value)
+        Deb822Dict.__setitem__(self, key, value)
+
 ###
 
 # XXX check what happens if input contains more that one signature
-- 
1.6.3.3

--
http://lists.alioth.debian.org/mailman/listinfo/pkg-python-debian-discuss

Reply via email to