Launchpad itself

Merge lp:~gary/launchpad/revert12041 into lp:launchpad

  1. revert12041
  2. Merge into devel
Proposed by Gary Poster
Status: Merged
Approved by: Francis J. Lacoste
Approved revision: no longer in the source branch.
Merged at revision: 12080
Proposed branch: lp:~gary/launchpad/revert12041
Merge into: lp:launchpad
Diff against target: 280 lines (+109/-16)
6 files modified
lib/canonical/launchpad/browser/librarian.py (+14/-13)
lib/canonical/launchpad/rest/bytestorage.py (+28/-0)
lib/canonical/launchpad/zcml/webservice.zcml (+13/-0)
lib/lp/bugs/interfaces/bugattachment.py (+2/-2)
lib/lp/bugs/stories/webservice/xx-bug.txt (+41/-1)
lib/lp/services/fields/__init__.py (+11/-0)
To merge this branch: bzr merge lp:~gary/launchpad/revert12041
Reviewer Review Type Date Requested Status
Francis J. Lacoste (community) Approve
Review via email: mp+43811@code.launchpad.net

Commit message

[r=flacoste][ui=none][no-qa] revert r12041, which is qa-bad and blocking deployment.

Description of the change

This reverts r12041, which is qa-bad and blocking deployment per the deployable revision report (https://devpad.canonical.com/~lpqateam/qa_reports/deployment-stable.html). No-one from the bugs team is around to confirm that what I am doing is correct, but it looks right to me from what I know of the situation.

Practically, this branch is merely the result of ``bzr merge -r12041..12040``. I did nothing else other than look at the output.

To post a comment you must log in.
Revision history for this message
Francis J. Lacoste (flacoste) wrote :

Looks sane

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/browser/librarian.py'
2--- lib/canonical/launchpad/browser/librarian.py 2010-12-07 11:00:09 +0000
3+++ lib/canonical/launchpad/browser/librarian.py 2010-12-15 19:14:18 +0000
4@@ -85,11 +85,11 @@
5
6 If the file is public, it will redirect to the files http url.
7
8- Otherwise if the feature flag publicrestrictedlibrarian is set to 'on'
9- this will allocate a token and redirect to the aliases private url.
10+ Otherwise if the feature flag publicrestrictedlibrarian is set to 'on' this
11+ will allocate a token and redirect to the aliases private url.
12
13 Otherwise it will proxy the file in the appserver.
14-
15+
16 Once we no longer have any proxy code at all it should be possible to
17 consolidate this with LibraryFileAliasView.
18
19@@ -98,7 +98,7 @@
20 we have to take special care about their origin.
21 SafeStreamOrRedirectLibraryFileAliasView is used when we do not trust the
22 content, otherwise StreamOrRedirectLibraryFileAliasView. We are working
23- to remove both of these views entirely, but some transition will be
24+ to remove both of these views entirely, but some transition will be
25 needed.
26
27 The context provides a file-like interface - it can be opened and closed
28@@ -161,7 +161,7 @@
29
30 def browserDefault(self, request):
31 """Decides how to deliver the file.
32-
33+
34 The options are:
35 - redirect to the contexts http url
36 - redirect to a time limited secure url
37@@ -182,7 +182,7 @@
38 # Public file, just point the client at the right place.
39 return RedirectionView(self.context.http_url, self.request), ()
40 if getFeatureFlag(u'publicrestrictedlibrarian') != 'on':
41- # Restricted file and we have not enabled the public
42+ # Restricted file and we have not enabled the public
43 # restricted librarian yet :- deliver inline.
44 self._when_streaming()
45 return self, ()
46@@ -203,12 +203,12 @@
47 """Hook for SafeStreamOrRedirectLibraryFileAliasView."""
48
49
50+
51 class SafeStreamOrRedirectLibraryFileAliasView(MixedFileAliasView):
52 """A view for Librarian files that sets the content disposition header."""
53
54 def _when_streaming(self):
55- super(
56- SafeStreamOrRedirectLibraryFileAliasView, self)._when_streaming()
57+ super(SafeStreamOrRedirectLibraryFileAliasView, self)._when_streaming()
58 self.request.response.setHeader(
59 'Content-Disposition', 'attachment')
60
61@@ -257,17 +257,18 @@
62 class ProxiedLibraryFileAlias:
63 """A `LibraryFileAlias` decorator for use in URL generation.
64
65- The URL's output by this decorator will always point at the webapp. This
66- is useful when:
67- - the webapp has to be contacted to get access to a file (required for
68- restricted files).
69+ The URL's output by this decorator will always point at the webapp. This is
70+ useful when:
71+ - we are proxying files via the webapp (as we do at the moment)
72+ - when the webapp has to be contacted to get access to a file (the case
73+ for restricted files in the future)
74 - files might change from public to private and thus not work even if the
75 user has access to the once its private, unless they go via the webapp.
76
77 This should be used anywhere we are outputting URL's to LibraryFileAliases
78 other than directly in rendered pages. For rendered pages, using a
79 LibraryFileAlias directly is OK as at that point the status of the file
80- is known.
81+ is know.
82
83 Overrides `ILibraryFileAlias.http_url` to always point to the webapp URL,
84 even when called from the webservice domain.
85
86=== modified file 'lib/canonical/launchpad/rest/bytestorage.py'
87--- lib/canonical/launchpad/rest/bytestorage.py 2010-11-18 18:42:05 +0000
88+++ lib/canonical/launchpad/rest/bytestorage.py 2010-12-15 19:14:18 +0000
89@@ -6,6 +6,7 @@
90 __metaclass__ = type
91 __all__ = [
92 'LibraryBackedByteStorage',
93+ 'RestrictedLibraryBackedByteStorage',
94 ]
95
96
97@@ -15,6 +16,7 @@
98 from zope.component import getUtility
99 from zope.interface import implements
100
101+from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
102 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
103 from canonical.launchpad.webapp.interfaces import ICanonicalUrlData
104
105@@ -73,3 +75,29 @@
106 def deleteStored(self):
107 """See `IByteStorage`."""
108 setattr(self.entry, self.field.__name__, None)
109+
110+
111+class RestrictedLibraryBackedByteStorage(LibraryBackedByteStorage):
112+ """See `IByteStorage`.
113+
114+ This variant of LibraryBackedByteStorage provides an alias_url
115+ which points to a StreamOrRedirectLibraryFileAliasView for
116+ restricted Librarian files.
117+ """
118+
119+ @property
120+ def alias_url(self):
121+ """See `IByteStorage`."""
122+ if self.file_alias.restricted:
123+ # XXX Abel Deuring 2010-09-03, bug=629804
124+ # This is a bad hack: We can't give ordinary users API access to
125+ # restricted files at present. But we can allow access to
126+ # some machines in the data center (and should do that).
127+ # We need here an HTTP URL, not an HTTPS URL, so we don't
128+ # use getUrl(). Since http_url uses not an official domain
129+ # name, all we do is expose an unaccessible HTTP URL also
130+ # users outside of the data center, instead of the HTTPS URL
131+ # returned by getURL().
132+ return self.file_alias.http_url
133+ else:
134+ return self.file_alias.getURL()
135
136=== modified file 'lib/canonical/launchpad/zcml/webservice.zcml'
137--- lib/canonical/launchpad/zcml/webservice.zcml 2010-11-18 18:42:05 +0000
138+++ lib/canonical/launchpad/zcml/webservice.zcml 2010-12-15 19:14:18 +0000
139@@ -43,6 +43,19 @@
140 <allow interface='lazr.restful.interfaces.IByteStorage' />
141 </class>
142
143+ <!-- Registration for the class that manages an entry's RestrictedBytes
144+ fields. -->
145+ <adapter
146+ for="lazr.restful.interfaces.IEntry
147+ lp.services.fields.IRestrictedBytes"
148+ provides="lazr.restful.interfaces.IByteStorage"
149+ factory="canonical.launchpad.rest.bytestorage.RestrictedLibraryBackedByteStorage"
150+ />
151+
152+ <class class="canonical.launchpad.rest.bytestorage.RestrictedLibraryBackedByteStorage">
153+ <allow interface='lazr.restful.interfaces.IByteStorage' />
154+ </class>
155+
156 <!-- WebService uses the default LaunchpadRootNavigation -->
157 <view
158 for="canonical.launchpad.interfaces.launchpad.IWebServiceApplication"
159
160=== modified file 'lib/lp/bugs/interfaces/bugattachment.py'
161--- lib/lp/bugs/interfaces/bugattachment.py 2010-11-18 18:42:05 +0000
162+++ lib/lp/bugs/interfaces/bugattachment.py 2010-12-15 19:14:18 +0000
163@@ -39,7 +39,7 @@
164 from canonical.launchpad import _
165 from canonical.launchpad.interfaces.launchpad import IHasBug
166 from canonical.launchpad.interfaces.message import IMessage
167-from lp.services.fields import Title
168+from lp.services.fields import RestrictedBytes, Title
169
170
171 class BugAttachmentType(DBEnumeratedType):
172@@ -115,7 +115,7 @@
173 libraryfile = Bytes(title=_("The attachment content."),
174 required=True)
175 data = exported(
176- Bytes(title=_("The attachment content."),
177+ RestrictedBytes(title=_("The attachment content."),
178 required=True,
179 readonly=True))
180 message = exported(
181
182=== modified file 'lib/lp/bugs/stories/webservice/xx-bug.txt'
183--- lib/lp/bugs/stories/webservice/xx-bug.txt 2010-12-07 11:00:09 +0000
184+++ lib/lp/bugs/stories/webservice/xx-bug.txt 2010-12-15 19:14:18 +0000
185@@ -1360,6 +1360,46 @@
186 ---
187
188
189+Attachments of private bugs
190+~~~~~~~~~~~~~~~~~~~~~~~~~~~
191+
192+Attachments of private bugs are served by the app server itself.
193+
194+ >>> print webservice.patch(
195+ ... bug_one['self_link'], 'application/json', dumps(dict(private=True)))
196+ HTTP/1.1 209 Content Returned...
197+
198+ >>> response = webservice.named_post(
199+ ... bug_one['self_link'], 'addAttachment',
200+ ... data="67890", filename="more-numbers.txt", content_type='foo/bar',
201+ ... comment="The numbers you asked for.")
202+ >>> attachments = webservice.get(
203+ ... bug_one['attachments_collection_link']).jsonBody()
204+ >>> attachment = attachments['entries'][0]
205+ >>> pprint_entry(attachment)
206+ bug_link:...
207+ data_link: u'http://api.launchpad.dev/beta/bugs/1/+attachment/.../data'
208+ ...
209+
210+XXX Abel Deuring 2010-09-03, bug=629804: Please note that the http URLs in this
211+example are part of a temporary hack. Please see the comment in
212+canonical.launchpad.rest.bytestorage.RestrictedLibraryBackedByteStorage.
213+alias_url for details.
214+
215+ >>> response = webservice.get(attachment['data_link'])
216+ >>> print response
217+ HTTP/1.1 303 See Other
218+ Status: 303 See Other
219+ ...
220+ Location: http://localhost:58005/94/more-numbers.txt
221+ ...
222+
223+ >>> # Let's make the bug public again for later tests.
224+ >>> print webservice.patch(
225+ ... bug_one['self_link'], 'application/json', dumps(dict(private=False)))
226+ HTTP/1.1 209 Content Returned...
227+
228+
229 Searching for bugs
230 ------------------
231
232@@ -2093,7 +2133,7 @@
233 next_collection_link: u'http://.../bugs/1/activity?ws.start=5&ws.size=5'
234 resource_type_link: u'http://.../#bug_activity-page-resource'
235 start: 0
236- total_size: 23
237+ total_size: 26
238 ...
239
240 >>> bug_nine_activity = webservice.get(
241
242=== modified file 'lib/lp/services/fields/__init__.py'
243--- lib/lp/services/fields/__init__.py 2010-12-07 11:00:09 +0000
244+++ lib/lp/services/fields/__init__.py 2010-12-15 19:14:18 +0000
245@@ -22,6 +22,7 @@
246 'ILocationField',
247 'INoneableTextLine',
248 'IPasswordField',
249+ 'IRestrictedBytes',
250 'IStrippedTextLine',
251 'ISummary',
252 'ITag',
253@@ -45,6 +46,7 @@
254 'ProductBugTracker',
255 'ProductNameField',
256 'PublicPersonChoice',
257+ 'RestrictedBytes',
258 'SearchTag',
259 'StrippedTextLine',
260 'Summary',
261@@ -227,6 +229,10 @@
262 """
263
264
265+class IRestrictedBytes(IBytes):
266+ """A marker interface used for restricted LibraryFileAlias fields."""
267+
268+
269 class StrippedTextLine(TextLine):
270 implements(IStrippedTextLine)
271
272@@ -836,3 +842,8 @@
273 else:
274 # The vocabulary prevents the revealing of private team names.
275 raise PrivateTeamNotAllowed(value)
276+
277+
278+class RestrictedBytes(Bytes):
279+ """A field for restricted LibraryFileAlias records."""
280+ implements(IRestrictedBytes)