3348: Crashes in RBTools 6.0 running against older RB installations (1.6.14 in my case)

IanMcC*******@gmai***** (Google Code) (Is this you? Claim this profile.)
chipx86
chipx86
June 9, 2014
What version are you running?

ReviewBoard itself: 1.6.14
RBTools: 6.0

What's the URL of the page containing the problem?

N/A -- It's a problem with RBTools

What steps will reproduce the problem?

1. try to use `rbt post` or `rbt status` from RBTools 6.0 against an older RB installation, 1.6.14 in my case

What is the expected output? What do you see instead?

Expected: 
Review request #XXXX posted.

https://rb.ipmcc.com/Project/XXXX/
https://rb.ipmcc.com/Project/XXXX/diff/

Observed: (with --debug)

>>> RBTools 0.6
>>> Python 2.7.5 (default, Aug 25 2013, 00:04:04) 
[GCC 4.2.1 Compatible Apple LLVM 5.0 (clang-500.0.68)]
>>> Running on Darwin-13.1.0-x86_64-i386-64bit
>>> Home = /Users/ipm
>>> Current directory = /Users/ipm/code/project/gtrunk
>>> Checking for a Subversion repository...
>>> Running: svn info --non-interactive
>>> Command exited with rc 1: ['svn', 'info', '--non-interactive']
svn: E155007: '/Users/ipm/code/project/gtrunk' is not a working copy
---
>>> Checking for a Git repository...
>>> Running: git rev-parse --git-dir
>>> Running: git config core.bare
>>> Running: git rev-parse --show-toplevel
>>> Running: git symbolic-ref -q HEAD
>>> Running: git config --get branch.movingLegend.merge
>>> Running: git config --get branch.movingLegend.remote
>>> Running: git config --get remote.origin.url
>>> repository info: Path: git@git.ipmcc.com:project.git, Base path: , Supports changesets: False
>>> Running: git config --get reviewboard.url
>>> Making HTTP GET request to https://rb.ipmcc.com/Project/api/
>>> Making HTTP GET request to https://rb.ipmcc.com/Project/api/info/
>>> Running: git rev-parse refs/heads/movingLegend
>>> Running: git merge-base c0342d4987792ccdb22d861d38379ed6a7fb3bbb origin/twigs/rbtest
>>> Running: git rev-parse 254a5b909aa168f74b7c23ffd57c76f735aef2a3
>>> Running: git status --porcelain --untracked-files=no
>>> Running: git diff --no-color --full-index --ignore-submodules --no-renames 254a5b909aa168f74b7c23ffd57c76f735aef2a3..c0342d4987792ccdb22d861d38379ed6a7fb3bbb
>>> Making HTTP GET request to https://rb.ipmcc.com/Project/api/review-requests/
>>> Making HTTP POST request to https://rb.ipmcc.com/Project/api/review-requests/
>>> Making HTTP GET request to https://rb.ipmcc.com/Project/api/review-requests/58649/diffs/
>>> Making HTTP POST request to https://rb.ipmcc.com/Project/api/review-requests/58649/diffs/
>>> Making HTTP GET request to https://rb.ipmcc.com/Project/api/review-requests/58649/draft/
>>> Making HTTP PUT request to https://rb.ipmcc.com/Project/api/review-requests/58649/draft/
Traceback (most recent call last):
  File "/usr/local/bin/rbt", line 8, in <module>
    load_entry_point('RBTools==0.6', 'console_scripts', 'rbt')()
  File "/Library/Python/2.7/site-packages/RBTools-0.6-py2.7.egg/rbtools/commands/main.py", line 134, in main
    command.run_from_argv([RB_MAIN, command_name] + args)
  File "/Library/Python/2.7/site-packages/RBTools-0.6-py2.7.egg/rbtools/commands/__init__.py", line 422, in run_from_argv
    exit_code = self.main(*args) or 0
  File "/Library/Python/2.7/site-packages/RBTools-0.6-py2.7.egg/rbtools/commands/post.py", line 769, in main
    submit_as=self.options.submit_as)
  File "/Library/Python/2.7/site-packages/RBTools-0.6-py2.7.egg/rbtools/commands/post.py", line 606, in post_request
    return review_request.id, review_request.absolute_url
  File "/Library/Python/2.7/site-packages/RBTools-0.6-py2.7.egg/rbtools/api/resource.py", line 284, in __getattr__
    raise AttributeError
AttributeError
hagar:gtrunk ipm$ 

What operating system are you using? What browser?

OS: MacOS 10.9.2
Browser: N/A 

Please provide any additional information below.

I've created a GitHub pull request that fixes these issues. The URL is: https://github.com/reviewboard/rbtools/pull/38

The `rbt post` crash is arguably just programmer error, although a bit confusing to diagnose. The ReviewRequestResource.absolute_url property provides fallback behavior for when the absolute_url field is not received in the server response, but that fallback implementation ends up raising because it tries to call self.url, which doesn't exist (and therefore raises an AttributeError for 'url'.) When you raise in a property.fget, Python handles it by swallowing that first exception, and falling back to calling __getattr__ for the name of the property, which then, itself, raises an AttributeError due to the missing absolute_url field. This kind of hides the "real" problem at first glance.

The `rbt status` crash is similar (missing property) but points to a *much* larger issue. When doing `rbt status` the code eventually calls repo.mirror_path, without checking to see if repo has a mirror_path attribute which, in our case, it does not. This is easily worked around with getattr (as in the pull request), but it points to a larger systemic problem. That is: RBTools takes data received over the network and effectively merges it into the type system of the python objects. Put differently, properties on python objects are defined (or not defined) based on responses from the server. Not only is this causing crashes in this instance, but it's a huge potential attack vector for malicious attacks (of the same ilk as SQL injection). The interface of the Python objects should be fixed/static regardless of network responses. If the objects need to vend a varying key/value set dependent on responses from the server, they should use a dictionary idiom, so it's clear to consumers that these properties are dynamic and not part of the type system.
chipx86
#1 chipx86
Fixed on release-0.6.x (5ecf53c)
  • +Fixed
  • +Component-RBTools
    +Milestone-RBTools-Release0.6.x
  • +chipx86