[users at bb.net] invalid origin bugfix

[Dave Vitek]

Hi all,

There is (or at least was in 0.9rc1) a type error in the error handling 
code for the "invalid origin" error.  If you tried to use a "force 
build" button from an invalid origin, it would cause this exception:

         Traceback (most recent call last):
           File 
"/usr/local/lib/python2.7/dist-packages/buildbot-0.9.0rc1-py2.7.egg/buildbot/www/rest.py", 
line 431, in render
             return self.asyncRenderHelper(request, self.asyncRender, 
writeError)
           File 
"/usr/local/lib/python2.7/dist-packages/buildbot-0.9.0rc1-py2.7.egg/buildbot/www/resource.py", 
line 83, in asyncRenderHelper
             @d.addErrback
           File 
"/usr/local/lib/python2.7/dist-packages/Twisted-16.2.0-py2.7-linux-x86_64.egg/twisted/internet/defer.py", 
line 328, in addErrback
             errbackKeywords=kw)
           File 
"/usr/local/lib/python2.7/dist-packages/Twisted-16.2.0-py2.7-linux-x86_64.egg/twisted/internet/defer.py", 
line 306, in addCallbacks
             self._runCallbacks()
         --- <exception caught here> ---
           File 
"/usr/local/lib/python2.7/dist-packages/Twisted-16.2.0-py2.7-linux-x86_64.egg/twisted/internet/defer.py", 
line 588, in _runCallbacks
             current.result = callback(current.result, *args, **kw)
           File 
"/usr/local/lib/python2.7/dist-packages/buildbot-0.9.0rc1-py2.7.egg/buildbot/www/resource.py", 
line 87, in failHttpError
             writeError(e.message, errcode=e.status)
           File 
"/usr/local/lib/python2.7/dist-packages/buildbot-0.9.0rc1-py2.7.egg/buildbot/www/rest.py", 
line 427, in writeError
             request.setResponseCode(errcode)
           File 
"/usr/local/lib/python2.7/dist-packages/Twisted-16.2.0-py2.7-linux-x86_64.egg/twisted/web/http.py", 
line 1059, in setResponseCode
             raise TypeError("HTTP response code must be int or long")
         exceptions.TypeError: HTTP response code must be int or long

Below is a patch that fixes the bug.  Basically, the problem was that 
errcode was the string '400' instead of the number 400.  It would likely 
be better to prevent errcode from being a string in the first place, if 
someone more familiar with the code knows how to do this.  However, it 
looks like the code raising the error does so with a number:
from twisted.web.error import Error
...
err = "invalid origin"
...
raise Error(400, err)

So maybe the unexpected int -> str conversion is somewhere inside twistd.

After having fixed that bug, I discovered that the UI, at least when 
using Chrome, completely failed to show any kind of feedback when an 
"invalid origin" error was correctly sent back to the browser.  The 
force build dialog just stayed open and did nothing when the button was 
clicked.  In light of this, I added the "if True" below so we would at 
least be able to look up errors in the log, even if the UI wouldn't show 
them.  This is more of a workaround for a UI bug.

Index: buildbot/buildbot/www/rest.py
===================================================================
--- buildbot/buildbot/www/rest.py	(revision 124703)
+++ buildbot/buildbot/www/rest.py	(revision 128749)
@@ -405,42 +405,53 @@
                  request.setHeader("content-length", len(data))
              else:
                  request.write(data)
  
      def reconfigResource(self, new_config):
          # buildbotURL may contain reverse proxy path, Origin header is just
          # scheme + host + port
          buildbotURL = urlparse(new_config.buildbotURL)
          origin_self = buildbotURL.scheme + "://" + buildbotURL.netloc
          # pre-translate the origin entries in the config
          self.origins = [re.compile(fnmatch.translate(o.lower()))
                          for o in new_config.www.get('allowed_origins',
                                                      [origin_self])]
  
          # and copy some other flags
          self.debug = new_config.www.get('debug')
          self.cache_seconds = new_config.www.get('json_cache_seconds', 0)
  
      def render(self, request):
          def writeError(msg, errcode=400):
-            if self.debug:
-                log.msg("HTTP error: %s" % (msg,))
+            # dvitek: Made this unconditional because buildbot's UI
+            # often does not show the error it gets back in a failed
+            # response!  For example, if you click force build and
+            # trigger an 'invalid origin' error.
+            if True or self.debug:
+                log.msg("HTTP error: %s: %s" % (repr(errcode), msg,))
+            # dvitek: Work around bug where errcode is a string but
+            # twistd wants a number!  The 'invalid origin' case seems
+            # to trigger this.
+            try:
+                errcode = int(errcode)
+            except ValueError:
+                errcode = 500
              request.setResponseCode(errcode)
              request.setHeader('content-type', 'text/plain; charset=utf-8')
              request.write(json.dumps(dict(error=msg)))
              request.finish()
          return self.asyncRenderHelper(request, self.asyncRender, writeError)
  
      @defer.inlineCallbacks
      def asyncRender(self, request):
  
          # Handle CORS, if necessary.
          origins = self.origins
          if origins is not None:
              isPreflight = False
              reqOrigin = request.getHeader('origin')
              if reqOrigin:
                  err = None
                  reqOrigin = reqOrigin.lower()
                  if not any(o.match(reqOrigin) for o in self.origins):
                      err = "invalid origin"
                  elif request.method == 'OPTIONS':



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.buildbot.net/pipermail/users/attachments/20160902/3d3a8fbe/attachment.html>