Restrict builder names to be unicode

buildbot · Sep 29, 2013 · d13c053 · d13c053
2 parents c5264eb + 9aaf540
commit d13c053
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 3 deletions.
diff --git a/master/buildbot/config.py b/master/buildbot/config.py
@@ -21,7 +21,7 @@
 import warnings
 from buildbot.util import safeTranslate
 from buildbot import interfaces
-from buildbot import locks
+from buildbot import locks, util
 from buildbot.revlinks import default_revlink_matcher
 from twisted.python import log, failure
 from twisted.internet import defer
@@ -701,7 +701,10 @@ def __init__(self, name=None, slavename=None, slavenames=None,
             name = '<unknown>'
         elif name[0] == '_':
             error("builder names must not start with an underscore: '%s'" % name)
-        self.name = name
+        try:
+            self.name = util.ascii2unicode(name)
+        except UnicodeDecodeError:
+            error("builder names must be unicode or ASCII")
 
         # factory is required
         if factory is None:

diff --git a/master/buildbot/process/builder.py b/master/buildbot/process/builder.py
@@ -46,7 +46,6 @@ class Builder(config.ReconfigurableServiceMixin,
     def __init__(self, name, _addServices=True):
         service.MultiService.__init__(self)
         self.name = name
-        self._builderid = None # filled in by getBuilderId
 
         # this is filled on demand by getBuilderId; don't access it directly
         self._builderid = None
@@ -118,6 +117,8 @@ def getBuilderId(self):
         # additional locking around this function.
         if self._builderid:
             return defer.succeed(self._builderid)
+        # buildbot.config should ensure this is already unicode, but it doesn't
+        # hurt to check again
         name = ascii2unicode(self.name)
         d = self.master.data.updates.findBuilderId(name)
         @d.addCallback

diff --git a/master/buildbot/test/unit/test_config.py b/master/buildbot/test/unit/test_config.py
@@ -1090,6 +1090,12 @@ def test_reserved_name(self):
             lambda : config.BuilderConfig(name='_a',
                 factory=self.factory, slavenames=['a']))
 
+    def test_utf8_name(self):
+        self.assertRaisesConfigError(
+            "builder names must be unicode or ASCII",
+            lambda : config.BuilderConfig(name=u"\N{SNOWMAN}".encode('utf-8'),
+                factory=self.factory, slavenames=['a']))
+
     def test_no_factory(self):
         self.assertRaisesConfigError(
             "builder 'a' has no factory",
@@ -1167,6 +1173,13 @@ def test_defaults(self):
             mergeRequests=None,
             description=None)
 
+    def test_unicode_name(self):
+        cfg = config.BuilderConfig(
+            name=u'a \N{SNOWMAN} c', slavename='a', factory=self.factory)
+        self.assertIdentical(cfg.factory, self.factory)
+        self.assertAttributes(cfg,
+            name=u'a \N{SNOWMAN} c')
+
     def test_args(self):
         cfg = config.BuilderConfig(
             name='b', slavename='s1', slavenames='s2', builddir='bd',

diff --git a/master/docs/relnotes/index.rst b/master/docs/relnotes/index.rst
@@ -62,6 +62,9 @@ Nine
 
 * SQLAlchemy-Migrate-0.6.1 is no longer supported.
 
+* Bulider names are now restricted to unicode strings or ASCII bytestrings.
+  Encoded bytestrings are not accepted.
+
 ..
     Any change that adds a feature or fixes a bug should have an entry here.
     Most simply need an additional bulleted list item, but more significant