Merge lp:~xtoddx/nova/imagedownload into lp:~hudson-openstack/nova/trunk

Proposed by Todd Willey
Status: Merged
Approved by: Jay Pipes
Approved revision: 368
Merged at revision: 373
Proposed branch: lp:~xtoddx/nova/imagedownload
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 107 lines (+13/-11)
1 file modified
nova/objectstore/handler.py (+13/-11)
To merge this branch: bzr merge lp:~xtoddx/nova/imagedownload
Reviewer Review Type Date Requested Status
Jay Pipes (community) Approve
Devin Carlen (community) Approve
Review via email: mp+38901@code.launchpad.net

Commit message

Authorize image access instead of just blindly giving it away.

To post a comment you must log in.
Revision history for this message
Devin Carlen (devcamcar) wrote :

lgtm

review: Approve
lp:~xtoddx/nova/imagedownload updated
368. By Todd Willey

Construct exception instead of raising a class.

Revision history for this message
Jay Pipes (jaypipes) wrote :

it's not necessary to use constructor parens when raising exceptions that take no arguments:

jpipes@serialcoder:~$ python
Python 2.6.5 (r265:79063, Apr 16 2010, 13:57:41)
[GCC 4.4.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> class SomeException(Exception):
... pass
...
>>> raise SomeException
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
__main__.SomeException
>>> raise SomeException()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
__main__.SomeException

That said, nothing wrong with the parens :)

lgtm.

review: Approve
Revision history for this message
Soren Hansen (soren) wrote :

Set a description and/or commit message, otherwise this'll never get merged.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/objectstore/handler.py'
2--- nova/objectstore/handler.py 2010-10-01 12:57:17 +0000
3+++ nova/objectstore/handler.py 2010-10-19 23:59:41 +0000
4@@ -119,7 +119,7 @@
5 # Authorization Header format: 'AWS <access>:<secret>'
6 authorization_header = request.getHeader('Authorization')
7 if not authorization_header:
8- raise exception.NotAuthorized
9+ raise exception.NotAuthorized()
10 auth_header_value = authorization_header.split(' ')[1]
11 access, _ignored, secret = auth_header_value.rpartition(':')
12 am = manager.AuthManager()
13@@ -134,7 +134,7 @@
14 return context.RequestContext(user, project)
15 except exception.Error as ex:
16 logging.debug("Authentication Failure: %s", ex)
17- raise exception.NotAuthorized
18+ raise exception.NotAuthorized()
19
20 class ErrorHandlingResource(resource.Resource):
21 """Maps exceptions to 404 / 401 codes. Won't work for
22@@ -209,7 +209,7 @@
23 return error.NoResource(message="No such bucket").render(request)
24
25 if not bucket_object.is_authorized(request.context):
26- raise exception.NotAuthorized
27+ raise exception.NotAuthorized()
28
29 prefix = get_argument(request, "prefix", u"")
30 marker = get_argument(request, "marker", u"")
31@@ -239,7 +239,7 @@
32 bucket_object = bucket.Bucket(self.name)
33
34 if not bucket_object.is_authorized(request.context):
35- raise exception.NotAuthorized
36+ raise exception.NotAuthorized()
37
38 bucket_object.delete()
39 request.setResponseCode(204)
40@@ -262,7 +262,7 @@
41 logging.debug("Getting object: %s / %s", self.bucket.name, self.name)
42
43 if not self.bucket.is_authorized(request.context):
44- raise exception.NotAuthorized
45+ raise exception.NotAuthorized()
46
47 obj = self.bucket[urllib.unquote(self.name)]
48 request.setHeader("Content-Type", "application/unknown")
49@@ -280,7 +280,7 @@
50 logging.debug("Putting object: %s / %s", self.bucket.name, self.name)
51
52 if not self.bucket.is_authorized(request.context):
53- raise exception.NotAuthorized
54+ raise exception.NotAuthorized()
55
56 key = urllib.unquote(self.name)
57 request.content.seek(0, 0)
58@@ -301,7 +301,7 @@
59 self.name)
60
61 if not self.bucket.is_authorized(request.context):
62- raise exception.NotAuthorized
63+ raise exception.NotAuthorized()
64
65 del self.bucket[urllib.unquote(self.name)]
66 request.setResponseCode(204)
67@@ -318,6 +318,8 @@
68
69 def render_GET(self, request):
70 """Returns the image file"""
71+ if not self.img.is_authorized(request.context, True):
72+ raise exception.NotAuthorized()
73 return static.File(self.img.image_path,
74 defaultType='application/octet-stream'
75 ).render_GET(request)
76@@ -369,12 +371,12 @@
77 image_path = os.path.join(FLAGS.images_path, image_id)
78 if not image_path.startswith(FLAGS.images_path) or \
79 os.path.exists(image_path):
80- raise exception.NotAuthorized
81+ raise exception.NotAuthorized()
82
83 bucket_object = bucket.Bucket(image_location.split("/")[0])
84
85 if not bucket_object.is_authorized(request.context):
86- raise exception.NotAuthorized
87+ raise exception.NotAuthorized()
88
89 p = multiprocessing.Process(target=image.Image.register_aws_image,
90 args=(image_id, image_location, request.context))
91@@ -389,7 +391,7 @@
92 image_object = image.Image(image_id)
93 if not image_object.is_authorized(request.context):
94 logging.debug("not authorized for render_POST in images")
95- raise exception.NotAuthorized
96+ raise exception.NotAuthorized()
97
98 operation = get_argument(request, 'operation', u'')
99 if operation:
100@@ -411,7 +413,7 @@
101 image_object = image.Image(image_id)
102
103 if not image_object.is_authorized(request.context):
104- raise exception.NotAuthorized
105+ raise exception.NotAuthorized()
106
107 image_object.delete()
108