[users at bb.net] invalid origin bugfix
Dave Vitek
dvitek at grammatech.com
Fri Sep 2 21:23:22 UTC 2016
My initial email was taking issue with the "HTTP response code must be
int or long" error. The "invalid origin" error was desirable for the
configuration.
The second bug (which I didn't fix) is in the reporting of the invalid
origin bug in the browser. After I fixed the first bug, so that the
server would send the "invalid origin" response back to the browser, the
UI still didn't communicate the error to the user. I would expect it to
say "invalid origin" somewhere on the screen.
- Dave
On 9/2/2016 5:10 PM, Pierre Tardy wrote:
>
> Indeed, this should work like this with *.
> Let us know if it doesn't.
>
> You open up yourself to XSS attacks though. This is why it's not the
> default setup.
>
>
> Le ven. 2 sept. 2016 23:05, Dave Vitek <dvitek at grammatech.com
> <mailto:dvitek at grammatech.com>> a écrit :
>
> Pierre,
>
> Is that to say that even with:
> allowed_origins=['*'],
>
> buildbot can only be used via a single hostname? This is somewhat
> less than ideal for people accessing buildbot through ssh tunnels
> and the like.
>
>
> - Dave
>
>
> On 9/2/2016 2:50 PM, Pierre Tardy wrote:
>> Hi Dave,
>>
>> Thanks for the report. Please note that we only accept patch
>> through github on the master branch.
>> You would have realized that this bug has been fixed in master
>> branch and has been released last week in version 0.9.0rc2.
>>
>> The problem you have is a misconfiguration of buildbotURL.
>> If you go on the home page of the UI, you will see a message that
>> tells you what the buildbotURL should look like.
>>
>> You are not the first one to have this issue. I think I will make
>> a patch which will make the UI completly refuse to work if the
>> URL is not setup correctly.
>>
>> Regards,
>> Pierre
>>
>> Le ven. 2 sept. 2016 à 18:25, Dave Vitek <dvitek at grammatech.com
>> <mailto:dvitek at grammatech.com>> a écrit :
>>
>> 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':
>>
>>
>>
>> _______________________________________________
>> users mailing list
>> users at buildbot.net <mailto:users at buildbot.net>
>> https://lists.buildbot.net/mailman/listinfo/users
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.buildbot.net/pipermail/users/attachments/20160902/58be6160/attachment.html>
More information about the users
mailing list