Merge lp:~jamesh/thumbnailer/use-aa-query-label into lp:thumbnailer/devel
- use-aa-query-label
- Merge into devel
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Approved by: | Michi Henning | ||||||||
Approved revision: | 226 | ||||||||
Merged at revision: | 218 | ||||||||
Proposed branch: | lp:~jamesh/thumbnailer/use-aa-query-label | ||||||||
Merge into: | lp:thumbnailer/devel | ||||||||
Diff against target: |
1060 lines (+321/-200) 22 files modified
CMakeLists.txt (+1/-0) include/internal/check_access.h (+42/-0) include/internal/thumbnailer.h (+4/-1) plugins/Ubuntu/Thumbnailer.0.1/CMakeLists.txt (+0/-1) plugins/Ubuntu/Thumbnailer.0.1/thumbnailgenerator.cpp (+1/-15) src/CMakeLists.txt (+2/-0) src/check_access.cpp (+83/-0) src/service/CMakeLists.txt (+1/-3) src/service/dbusinterface.cpp (+1/-2) src/service/dbusinterface.h (+0/-1) src/service/dbusinterface.xml (+1/-2) src/service/handler.cpp (+1/-1) src/thumbnailer-admin/get_local_thumbnail.cpp (+1/-13) src/thumbnailer.cpp (+41/-26) tests/CMakeLists.txt (+1/-0) tests/check_access/CMakeLists.txt (+3/-0) tests/check_access/check_access_test.cpp (+64/-0) tests/dbus/dbus_test.cpp (+5/-42) tests/qml/tst_albumart.qml (+4/-12) tests/testsetup.h.in (+1/-1) tests/thumbnailer-admin/thumbnailer-admin_test.cpp (+2/-2) tests/thumbnailer/thumbnailer_test.cpp (+62/-78) |
||||||||
To merge this branch: | bzr merge lp:~jamesh/thumbnailer/use-aa-query-label | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot (community) | continuous-integration | Approve | |
Michi Henning (community) | Approve | ||
Review via email:
|
Commit message
Remove the filename_fd argument to the GetThumbnail() D-Bus method, instead using aa_query_label() to check whether the caller has access to the relevant file.
Description of the change
Update the GetThumbnail dbus method to remove the filename_fd argument. Instead, use aa_query_label() to check whether the client has access to the target file.
We were already canonicalising the path, so shouldn't have trouble getting tricked by symlinks.
I'll provide some more concrete ways to test this on the phone once I've got ARM binaries.

PS Jenkins bot (ps-jenkins) wrote : | # |

PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:223
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https:/
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://

PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:224
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https:/
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://

James Henstridge (jamesh) wrote : | # |
Here is the steps I went to test this on the phone after installing it:
1. run "sudo aa-status" to find the list of profiles installed on the system. I looked for the label for the gallery app, which in this case was "com.ubuntu.
2. Copy an image to ~/.local/
3. Change to ~/.local/
4. Run the following command (adjusting for exact label name):
aa-exec -p com.ubuntu.
I then received the following error message:
thumbnailer-admin: checkFinished(): thumbnail: /home/phablet/
LocalThumbn
(plus a bunch of output complaining that it couldn't write to .gcda files, since this was a coverage build).

PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
No commit message was specified in the merge proposal. Hit 'Add commit message' on the merge proposal web page or follow the link below. You can approve the merge proposal yourself to rerun.
https:/
- 225. By James Henstridge
-
Add some more debug logging.

PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:225
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https:/
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 226. By James Henstridge
-
Remove the extra nul byte at the end of the buffer passed to
aa_query_label() -- this causes the check to fail for AppArmor labels
other than "unconfined".

PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:226
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https:/
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://

Michi Henning (michihenning) wrote : | # |
Excellent, thank you!

PS Jenkins bot (ps-jenkins) : | # |
Preview Diff
1 | === modified file 'CMakeLists.txt' |
2 | --- CMakeLists.txt 2015-06-12 04:38:11 +0000 |
3 | +++ CMakeLists.txt 2015-06-17 06:15:32 +0000 |
4 | @@ -104,6 +104,7 @@ |
5 | include_directories(${GIO_DEPS_INCLUDE_DIRS}) |
6 | include_directories(${IMG_DEPS_INCLUDE_DIRS}) |
7 | include_directories(${UNITY_API_DEPS_INCLUDE_DIRS}) |
8 | +include_directories(${APPARMOR_DEPS_INCLUDE_DIRS}) |
9 | include_directories(include) |
10 | include_directories(${CMAKE_CURRENT_BINARY_DIR}/include) |
11 | |
12 | |
13 | === added file 'include/internal/check_access.h' |
14 | --- include/internal/check_access.h 1970-01-01 00:00:00 +0000 |
15 | +++ include/internal/check_access.h 2015-06-17 06:15:32 +0000 |
16 | @@ -0,0 +1,42 @@ |
17 | +/* |
18 | + * Copyright (C) 2015 Canonical Ltd. |
19 | + * |
20 | + * This program is free software: you can redistribute it and/or modify |
21 | + * it under the terms of the GNU General Public License version 3 as |
22 | + * published by the Free Software Foundation. |
23 | + * |
24 | + * This program is distributed in the hope that it will be useful, |
25 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
26 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
27 | + * GNU General Public License for more details. |
28 | + * |
29 | + * You should have received a copy of the GNU General Public License |
30 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
31 | + * |
32 | + * Authored by: James Henstridge <james.henstridge@canonical.com> |
33 | + */ |
34 | + |
35 | +#pragma once |
36 | + |
37 | +#include <string> |
38 | + |
39 | +namespace unity |
40 | +{ |
41 | + |
42 | +namespace thumbnailer |
43 | +{ |
44 | + |
45 | +namespace internal |
46 | +{ |
47 | + |
48 | +// Returns true if the given AppArmor profile can read the given path. |
49 | +// Note that if path is a symlink, the check will be against the |
50 | +// symlink path rather than the symlink target. |
51 | +bool apparmor_can_read(std::string const& apparmor_label, |
52 | + std::string const& path); |
53 | + |
54 | +} // namespace internal |
55 | + |
56 | +} // namespace thumbnailer |
57 | + |
58 | +} // namespace unity |
59 | |
60 | === modified file 'include/internal/thumbnailer.h' |
61 | --- include/internal/thumbnailer.h 2015-06-08 06:03:00 +0000 |
62 | +++ include/internal/thumbnailer.h 2015-06-17 06:15:32 +0000 |
63 | @@ -72,6 +72,10 @@ |
64 | virtual FetchStatus status() const = 0; |
65 | |
66 | virtual std::string const& key() const = 0; |
67 | + |
68 | + // Set the credentials of the caller. |
69 | + virtual void set_client_credentials(uid_t user, std::string const& apparmor_label) = 0; |
70 | + |
71 | Q_SIGNALS: |
72 | void downloadFinished(); |
73 | }; |
74 | @@ -103,7 +107,6 @@ |
75 | * If the thumbnail could not be generated, an empty string is returned. |
76 | */ |
77 | std::unique_ptr<ThumbnailRequest> get_thumbnail(std::string const& filename, |
78 | - int filename_fd, |
79 | QSize const& requested_size); |
80 | |
81 | /** |
82 | |
83 | === modified file 'plugins/Ubuntu/Thumbnailer.0.1/CMakeLists.txt' |
84 | --- plugins/Ubuntu/Thumbnailer.0.1/CMakeLists.txt 2015-06-08 01:34:57 +0000 |
85 | +++ plugins/Ubuntu/Thumbnailer.0.1/CMakeLists.txt 2015-06-17 06:15:32 +0000 |
86 | @@ -10,7 +10,6 @@ |
87 | albumartgenerator.cpp |
88 | artgeneratorcommon.cpp |
89 | artistartgenerator.cpp |
90 | - ${CMAKE_SOURCE_DIR}/src/safe_strerror.cpp |
91 | plugin.cpp |
92 | thumbnailerimageresponse.cpp |
93 | thumbnailgenerator.cpp |
94 | |
95 | === modified file 'plugins/Ubuntu/Thumbnailer.0.1/thumbnailgenerator.cpp' |
96 | --- plugins/Ubuntu/Thumbnailer.0.1/thumbnailgenerator.cpp 2015-06-11 10:21:50 +0000 |
97 | +++ plugins/Ubuntu/Thumbnailer.0.1/thumbnailgenerator.cpp 2015-06-17 06:15:32 +0000 |
98 | @@ -22,11 +22,6 @@ |
99 | #include <service/dbus_names.h> |
100 | #include "thumbnailerimageresponse.h" |
101 | |
102 | -#include <internal/safe_strerror.h> |
103 | - |
104 | -#include <fcntl.h> |
105 | -#include <unistd.h> |
106 | - |
107 | namespace |
108 | { |
109 | |
110 | @@ -77,15 +72,6 @@ |
111 | * cases we don't want to do that for performance reasons, so this |
112 | * is the only way around the issue for now. */ |
113 | QString src_path = QUrl(id).path(); |
114 | - int fd = open(src_path.toUtf8().constData(), O_RDONLY | O_CLOEXEC); |
115 | - if (fd < 0) |
116 | - { |
117 | - qDebug() << "ThumbnailGenerator::requestImageResponse(): cannot open " + src_path + ": " + |
118 | - QString::fromStdString(internal::safe_strerror(errno)); |
119 | - return new ThumbnailerImageResponse(requestedSize, default_image_based_on_mime(id)); |
120 | - } |
121 | - QDBusUnixFileDescriptor unix_fd(fd); |
122 | - close(fd); |
123 | |
124 | if (!connection) |
125 | { |
126 | @@ -95,7 +81,7 @@ |
127 | iface.reset(new ThumbnailerInterface(service::BUS_NAME, service::THUMBNAILER_BUS_PATH, *connection)); |
128 | } |
129 | |
130 | - auto reply = iface->GetThumbnail(src_path, unix_fd, requestedSize); |
131 | + auto reply = iface->GetThumbnail(src_path, requestedSize); |
132 | std::unique_ptr<QDBusPendingCallWatcher> watcher( |
133 | new QDBusPendingCallWatcher(reply)); |
134 | return new ThumbnailerImageResponse(requestedSize, default_image_based_on_mime(id), std::move(watcher)); |
135 | |
136 | === modified file 'src/CMakeLists.txt' |
137 | --- src/CMakeLists.txt 2015-06-11 09:34:31 +0000 |
138 | +++ src/CMakeLists.txt 2015-06-17 06:15:32 +0000 |
139 | @@ -13,6 +13,7 @@ |
140 | |
141 | add_library(thumbnailer STATIC |
142 | artdownloader.cpp |
143 | + check_access.cpp |
144 | file_io.cpp |
145 | image.cpp |
146 | imageextractor.cpp |
147 | @@ -40,6 +41,7 @@ |
148 | ${GIO_DEPS_LDFLAGS} |
149 | ${IMG_DEPS_LDFLAGS} |
150 | ${UNITY_API_DEPS_LDFLAGS} |
151 | + ${APPARMOR_DEPS_LDFLAGS} |
152 | ) |
153 | |
154 | add_subdirectory(service) |
155 | |
156 | === added file 'src/check_access.cpp' |
157 | --- src/check_access.cpp 1970-01-01 00:00:00 +0000 |
158 | +++ src/check_access.cpp 2015-06-17 06:15:32 +0000 |
159 | @@ -0,0 +1,83 @@ |
160 | +/* |
161 | + * Copyright (C) 2015 Canonical Ltd. |
162 | + * |
163 | + * This program is free software: you can redistribute it and/or modify |
164 | + * it under the terms of the GNU General Public License version 3 as |
165 | + * published by the Free Software Foundation. |
166 | + * |
167 | + * This program is distributed in the hope that it will be useful, |
168 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
169 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
170 | + * GNU General Public License for more details. |
171 | + * |
172 | + * You should have received a copy of the GNU General Public License |
173 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
174 | + * |
175 | + * Authored by: James Henstridge <james.henstridge@canonical.com> |
176 | + */ |
177 | + |
178 | +#include <internal/check_access.h> |
179 | +#include <internal/safe_strerror.h> |
180 | + |
181 | +#include <errno.h> |
182 | +#include <stdexcept> |
183 | +#include <sys/apparmor.h> |
184 | + |
185 | +using namespace std; |
186 | + |
187 | +namespace |
188 | +{ |
189 | + |
190 | +enum class Access |
191 | +{ |
192 | + write = 1, |
193 | + read = 2 |
194 | +}; |
195 | + |
196 | +char const AA_CLASS_FILE = 2; |
197 | + |
198 | +bool query_file(Access access, string const& label, string const& path) |
199 | +{ |
200 | + static bool enabled = aa_is_enabled(); |
201 | + if (!enabled) |
202 | + { |
203 | + // If AppArmor is not enabled, assume access is granted. |
204 | + return true; |
205 | + } |
206 | + |
207 | + string query(AA_QUERY_CMD_LABEL, AA_QUERY_CMD_LABEL_SIZE); |
208 | + query += label; |
209 | + query += '\0'; |
210 | + query += AA_CLASS_FILE; |
211 | + query += path; |
212 | + |
213 | + int allowed = 0, audited = 0; |
214 | + if (aa_query_label(static_cast<uint32_t>(access), const_cast<char*>(query.data()), query.size(), &allowed, &audited) < 0) |
215 | + { |
216 | + using namespace unity::thumbnailer::internal; |
217 | + throw runtime_error("query_file(): Could not query AppArmor access: " + safe_strerror(errno)); |
218 | + } |
219 | + return allowed; |
220 | +} |
221 | + |
222 | +} |
223 | + |
224 | +namespace unity |
225 | +{ |
226 | + |
227 | +namespace thumbnailer |
228 | +{ |
229 | + |
230 | +namespace internal |
231 | +{ |
232 | + |
233 | +bool apparmor_can_read(string const& apparmor_label, string const& path) |
234 | +{ |
235 | + return query_file(Access::read, apparmor_label, path); |
236 | +} |
237 | + |
238 | +} // namespace internal |
239 | + |
240 | +} // namespace thumbnailer |
241 | + |
242 | +} // namespace unity |
243 | |
244 | === modified file 'src/service/CMakeLists.txt' |
245 | --- src/service/CMakeLists.txt 2015-06-12 04:38:11 +0000 |
246 | +++ src/service/CMakeLists.txt 2015-06-17 06:15:32 +0000 |
247 | @@ -1,5 +1,3 @@ |
248 | -add_definitions(${APPARMOR_DEPS_CFLAGS}) |
249 | - |
250 | qt5_add_dbus_adaptor(adaptor_files dbusinterface.xml dbusinterface.h unity::thumbnailer::service::DBusInterface) |
251 | qt5_add_dbus_adaptor(adaptor_files admininterface.xml admininterface.h unity::thumbnailer::service::AdminInterface) |
252 | |
253 | @@ -21,7 +19,7 @@ |
254 | ) |
255 | |
256 | qt5_use_modules(thumbnailer-service DBus Concurrent) |
257 | -target_link_libraries(thumbnailer-service thumbnailer ${CMAKE_THREAD_LIBS_INIT} ${APPARMOR_DEPS_LDFLAGS}) |
258 | +target_link_libraries(thumbnailer-service thumbnailer) |
259 | set_target_properties(thumbnailer-service PROPERTIES AUTOMOC TRUE) |
260 | add_dependencies(thumbnailer-service vs-thumb) |
261 | |
262 | |
263 | === modified file 'src/service/dbusinterface.cpp' |
264 | --- src/service/dbusinterface.cpp 2015-06-15 03:51:27 +0000 |
265 | +++ src/service/dbusinterface.cpp 2015-06-17 06:15:32 +0000 |
266 | @@ -114,7 +114,6 @@ |
267 | } |
268 | |
269 | QDBusUnixFileDescriptor DBusInterface::GetThumbnail(QString const& filename, |
270 | - QDBusUnixFileDescriptor const& filename_fd, |
271 | QSize const& requestedSize) |
272 | { |
273 | std::unique_ptr<ThumbnailRequest> request; |
274 | @@ -124,7 +123,7 @@ |
275 | QString details; |
276 | QTextStream s(&details); |
277 | s << "thumbnail: " << filename << " (" << requestedSize.width() << "," << requestedSize.height() << ")"; |
278 | - auto request = thumbnailer_->get_thumbnail(filename.toStdString(), filename_fd.fileDescriptor(), requestedSize); |
279 | + auto request = thumbnailer_->get_thumbnail(filename.toStdString(), requestedSize); |
280 | queueRequest(new Handler(connection(), message(), |
281 | check_thread_pool_, create_thread_pool_, |
282 | extraction_limiter_, credentials(), |
283 | |
284 | === modified file 'src/service/dbusinterface.h' |
285 | --- src/service/dbusinterface.h 2015-06-12 06:49:20 +0000 |
286 | +++ src/service/dbusinterface.h 2015-06-17 06:15:32 +0000 |
287 | @@ -52,7 +52,6 @@ |
288 | QDBusUnixFileDescriptor GetAlbumArt(QString const& artist, QString const& album, QSize const& requestedSize); |
289 | QDBusUnixFileDescriptor GetArtistArt(QString const& artist, QString const& album, QSize const& requestedSize); |
290 | QDBusUnixFileDescriptor GetThumbnail(QString const& filename, |
291 | - QDBusUnixFileDescriptor const& filename_fd, |
292 | QSize const& requestedSize); |
293 | |
294 | private: |
295 | |
296 | === modified file 'src/service/dbusinterface.xml' |
297 | --- src/service/dbusinterface.xml 2015-05-20 00:58:20 +0000 |
298 | +++ src/service/dbusinterface.xml 2015-06-17 06:15:32 +0000 |
299 | @@ -18,10 +18,9 @@ |
300 | </method> |
301 | <method name="GetThumbnail"> |
302 | <arg direction="in" type="s" name="filename" /> |
303 | - <arg direction="in" type="h" name="filename_fd" /> |
304 | <arg direction="in" type="(ii)" name="requestedSize" /> |
305 | <arg direction="out" type="h" name="fd" /> |
306 | - <annotation name="org.qtproject.QtDBus.QtTypeName.In2" value="QSize" /> |
307 | + <annotation name="org.qtproject.QtDBus.QtTypeName.In1" value="QSize" /> |
308 | <annotation name="org.gtk.GDBus.C.UnixFD" value="true" /> |
309 | </method> |
310 | </interface> |
311 | |
312 | === modified file 'src/service/handler.cpp' |
313 | --- src/service/handler.cpp 2015-06-15 04:16:34 +0000 |
314 | +++ src/service/handler.cpp 2015-06-17 06:15:32 +0000 |
315 | @@ -201,7 +201,7 @@ |
316 | sendError("gotCredentials(): " + details() + ": could not retrieve peer credentials"); |
317 | return; |
318 | } |
319 | - qDebug() << "Peer" << p->message.service() << "has uid =" << credentials.user << "label =" << QString::fromStdString(credentials.label); |
320 | + p->request->set_client_credentials(credentials.user, credentials.label); |
321 | |
322 | auto do_check = [this]() -> FdOrError |
323 | { |
324 | |
325 | === modified file 'src/thumbnailer-admin/get_local_thumbnail.cpp' |
326 | --- src/thumbnailer-admin/get_local_thumbnail.cpp 2015-06-03 22:24:53 +0000 |
327 | +++ src/thumbnailer-admin/get_local_thumbnail.cpp 2015-06-17 06:15:32 +0000 |
328 | @@ -20,7 +20,6 @@ |
329 | |
330 | #include "parse_size.h" |
331 | #include <internal/file_io.h> |
332 | -#include <internal/raii.h> |
333 | #include <internal/safe_strerror.h> |
334 | #include "util.h" |
335 | |
336 | @@ -90,18 +89,7 @@ |
337 | { |
338 | try |
339 | { |
340 | - QDBusUnixFileDescriptor ufd; |
341 | - { |
342 | - FdPtr fd(do_close); |
343 | - fd.reset(open(input_path_.toUtf8().data(), O_RDONLY)); |
344 | - if (fd.get() == -1) |
345 | - { |
346 | - throw QString("GetLocalThumbnail::run(): cannot open ") + input_path_ + ": " + |
347 | - QString::fromStdString(safe_strerror(errno)); |
348 | - } |
349 | - ufd.setFileDescriptor(fd.get()); |
350 | - } |
351 | - auto reply = conn.thumbnailer().GetThumbnail(input_path_, ufd, size_); |
352 | + auto reply = conn.thumbnailer().GetThumbnail(input_path_, size_); |
353 | reply.waitForFinished(); |
354 | if (!reply.isValid()) |
355 | { |
356 | |
357 | === modified file 'src/thumbnailer.cpp' |
358 | --- src/thumbnailer.cpp 2015-06-11 09:34:31 +0000 |
359 | +++ src/thumbnailer.cpp 2015-06-17 06:15:32 +0000 |
360 | @@ -21,6 +21,7 @@ |
361 | #include <internal/thumbnailer.h> |
362 | |
363 | #include <internal/artreply.h> |
364 | +#include <internal/check_access.h> |
365 | #include <internal/image.h> |
366 | #include <internal/imageextractor.h> |
367 | #include <internal/make_directories.h> |
368 | @@ -76,6 +77,11 @@ |
369 | { |
370 | return key_; |
371 | } |
372 | + void set_client_credentials(uid_t user, std::string const& label) override |
373 | + { |
374 | + client_user_ = user; |
375 | + client_apparmor_label_ = label; |
376 | + } |
377 | enum class CachePolicy |
378 | { |
379 | cache_fullsize, |
380 | @@ -138,6 +144,8 @@ |
381 | string key_; |
382 | QSize const requested_size_; |
383 | chrono::milliseconds timeout_; |
384 | + uid_t client_user_ = 0; |
385 | + string client_apparmor_label_; |
386 | |
387 | private: |
388 | FetchStatus status_; |
389 | @@ -152,7 +160,6 @@ |
390 | public: |
391 | LocalThumbnailRequest(Thumbnailer* thumbnailer, |
392 | string const& filename, |
393 | - int filename_fd, |
394 | QSize const& requested_size, |
395 | chrono::milliseconds timeout); |
396 | |
397 | @@ -371,36 +378,24 @@ |
398 | |
399 | LocalThumbnailRequest::LocalThumbnailRequest(Thumbnailer* thumbnailer, |
400 | string const& filename, |
401 | - int filename_fd, |
402 | QSize const& requested_size, |
403 | chrono::milliseconds timeout) |
404 | : RequestBase(thumbnailer, "", requested_size, timeout) |
405 | , filename_(filename) |
406 | , fd_(-1, do_close) |
407 | { |
408 | + // We canonicalise the path name both to avoid caching the file |
409 | + // multiple times, and to ensure our access checks are against the |
410 | + // real file rather than a symlink. |
411 | filename_ = boost::filesystem::canonical(filename).native(); |
412 | - fd_.reset(open(filename_.c_str(), O_RDONLY | O_CLOEXEC)); |
413 | - if (fd_.get() < 0) |
414 | - { |
415 | - // LCOV_EXCL_START |
416 | - throw runtime_error("LocalThumbnailRequest(): Could not open " + filename_ + ": " + safe_strerror(errno)); |
417 | - // LCOV_EXCL_STOP |
418 | - } |
419 | - struct stat our_stat, client_stat; |
420 | - if (fstat(fd_.get(), &our_stat) < 0) |
421 | + |
422 | + struct stat st; |
423 | + if (stat(filename_.c_str(), &st) < 0) |
424 | { |
425 | // LCOV_EXCL_START |
426 | throw runtime_error("LocalThumbnailRequest(): Could not stat " + filename_ + ": " + safe_strerror(errno)); |
427 | // LCOV_EXCL_STOP |
428 | } |
429 | - if (fstat(filename_fd, &client_stat) < 0) |
430 | - { |
431 | - throw runtime_error("LocalThumbnailRequest(): Could not stat file descriptor: " + safe_strerror(errno)); |
432 | - } |
433 | - if (our_stat.st_dev != client_stat.st_dev || our_stat.st_ino != client_stat.st_ino) |
434 | - { |
435 | - throw runtime_error("LocalThumbnailRequest(): file descriptor does not refer to file " + filename_); |
436 | - } |
437 | |
438 | // The full cache key for the file is the concatenation of path |
439 | // name, inode, modification time, and inode modification time |
440 | @@ -413,11 +408,11 @@ |
441 | // due to file removal or file update are rare). |
442 | key_ = filename_; |
443 | key_ += '\0'; |
444 | - key_ += to_string(our_stat.st_ino); |
445 | - key_ += '\0'; |
446 | - key_ += to_string(our_stat.st_mtim.tv_sec) + "." + to_string(our_stat.st_mtim.tv_nsec); |
447 | - key_ += '\0'; |
448 | - key_ += to_string(our_stat.st_ctim.tv_sec) + "." + to_string(our_stat.st_ctim.tv_nsec); |
449 | + key_ += to_string(st.st_ino); |
450 | + key_ += '\0'; |
451 | + key_ += to_string(st.st_mtim.tv_sec) + "." + to_string(st.st_mtim.tv_nsec); |
452 | + key_ += '\0'; |
453 | + key_ += to_string(st.st_ctim.tv_sec) + "." + to_string(st.st_ctim.tv_nsec); |
454 | } |
455 | |
456 | RequestBase::ImageData LocalThumbnailRequest::fetch(QSize const& size_hint) |
457 | @@ -428,6 +423,19 @@ |
458 | return ImageData(Image(image_extractor_->data()), CachePolicy::cache_fullsize, Location::local); |
459 | } |
460 | |
461 | + if (client_user_ != geteuid()) |
462 | + { |
463 | + // LCOV_EXCL_START |
464 | + throw runtime_error("LocalThumbnailRequest::fetch(): Request comes from a different user ID"); |
465 | + // LCOV_EXCL_STOP |
466 | + } |
467 | + if (!apparmor_can_read(client_apparmor_label_, filename_)) { |
468 | + // LCOV_EXCL_START |
469 | + qDebug() << "Apparmor label" << QString::fromStdString(client_apparmor_label_) << "has no access to" << QString::fromStdString(filename_); |
470 | + throw runtime_error("LocalThumbnailRequest::fetch(): AppArmor policy forbids access to " + filename_); |
471 | + // LCOV_EXCL_STOP |
472 | + } |
473 | + |
474 | // Work out content type. |
475 | gobj_ptr<GFile> file(g_file_new_for_path(filename_.c_str())); |
476 | assert(file); // Cannot fail according to doc. |
477 | @@ -447,6 +455,14 @@ |
478 | return ImageData(FetchStatus::error, Location::local); // LCOV_EXCL_LINE |
479 | } |
480 | |
481 | + fd_.reset(open(filename_.c_str(), O_RDONLY | O_CLOEXEC)); |
482 | + if (fd_.get() < 0) |
483 | + { |
484 | + // LCOV_EXCL_START |
485 | + throw runtime_error("LocalThumbnailRequest(): Could not open " + filename_ + ": " + safe_strerror(errno)); |
486 | + // LCOV_EXCL_STOP |
487 | + } |
488 | + |
489 | // Call the appropriate image extractor and return the image data as JPEG (not scaled). |
490 | // We indicate that full-size images are to be cached only for audio and video files, |
491 | // for which extraction is expensive. For local images, we don't cache full size. |
492 | @@ -621,7 +637,6 @@ |
493 | Thumbnailer::~Thumbnailer() = default; |
494 | |
495 | unique_ptr<ThumbnailRequest> Thumbnailer::get_thumbnail(string const& filename, |
496 | - int filename_fd, |
497 | QSize const& requested_size) |
498 | { |
499 | assert(!filename.empty()); |
500 | @@ -629,7 +644,7 @@ |
501 | try |
502 | { |
503 | return unique_ptr<ThumbnailRequest>( |
504 | - new LocalThumbnailRequest(this, filename, filename_fd, requested_size, extraction_timeout_)); |
505 | + new LocalThumbnailRequest(this, filename, requested_size, extraction_timeout_)); |
506 | } |
507 | catch (std::exception const&) |
508 | { |
509 | |
510 | === modified file 'tests/CMakeLists.txt' |
511 | --- tests/CMakeLists.txt 2015-06-06 08:23:29 +0000 |
512 | +++ tests/CMakeLists.txt 2015-06-17 06:15:32 +0000 |
513 | @@ -8,6 +8,7 @@ |
514 | add_subdirectory(utils) |
515 | |
516 | set(unit_test_dirs |
517 | + check_access |
518 | dbus |
519 | download |
520 | file_io |
521 | |
522 | === added directory 'tests/check_access' |
523 | === added file 'tests/check_access/CMakeLists.txt' |
524 | --- tests/check_access/CMakeLists.txt 1970-01-01 00:00:00 +0000 |
525 | +++ tests/check_access/CMakeLists.txt 2015-06-17 06:15:32 +0000 |
526 | @@ -0,0 +1,3 @@ |
527 | +add_executable(check_access_test check_access_test.cpp) |
528 | +target_link_libraries(check_access_test thumbnailer gtest) |
529 | +add_test(check_access check_access_test) |
530 | |
531 | === added file 'tests/check_access/check_access_test.cpp' |
532 | --- tests/check_access/check_access_test.cpp 1970-01-01 00:00:00 +0000 |
533 | +++ tests/check_access/check_access_test.cpp 2015-06-17 06:15:32 +0000 |
534 | @@ -0,0 +1,64 @@ |
535 | +/* |
536 | + * Copyright (C) 2015 Canonical Ltd. |
537 | + * |
538 | + * This program is free software: you can redistribute it and/or modify |
539 | + * it under the terms of the GNU General Public License version 3 as |
540 | + * published by the Free Software Foundation. |
541 | + * |
542 | + * This program is distributed in the hope that it will be useful, |
543 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
544 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
545 | + * GNU General Public License for more details. |
546 | + * |
547 | + * You should have received a copy of the GNU General Public License |
548 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
549 | + * |
550 | + * Authored by: James Henstridge <james.henstridge@canonical.com> |
551 | + */ |
552 | + |
553 | +#include <internal/check_access.h> |
554 | + |
555 | +#include <gtest/gtest.h> |
556 | +#include <testsetup.h> |
557 | + |
558 | +#include <cstdio> |
559 | +#include <errno.h> |
560 | +#include <stdexcept> |
561 | +#include <sys/apparmor.h> |
562 | + |
563 | +using namespace std; |
564 | +using namespace unity::thumbnailer::internal; |
565 | + |
566 | +TEST(check_acess, unconfined) |
567 | +{ |
568 | + if (!aa_is_enabled()) { |
569 | + printf("AppArmor is disabled\n"); |
570 | + return; |
571 | + } |
572 | + |
573 | + EXPECT_TRUE(apparmor_can_read("unconfined", "/etc/passwd")); |
574 | +} |
575 | + |
576 | +TEST(check_access, confined) { |
577 | + if (!aa_is_enabled()) { |
578 | + printf("AppArmor is disabled\n"); |
579 | + return; |
580 | + } |
581 | + |
582 | + // We can't load new profiles into the kernel from the tests, so |
583 | + // try one of the base system profiles that is probably loaded. |
584 | + try |
585 | + { |
586 | + EXPECT_FALSE(apparmor_can_read("/sbin/dhclient", "/etc/passwd")); |
587 | + } |
588 | + catch (std::runtime_error const&) { |
589 | + EXPECT_EQ(ENOENT, errno); |
590 | + printf("Test AppArmor label not loaded in kernel\n"); |
591 | + } |
592 | +} |
593 | + |
594 | +int main(int argc, char** argv) |
595 | +{ |
596 | + ::testing::InitGoogleTest(&argc, argv); |
597 | + return RUN_ALL_TESTS(); |
598 | +} |
599 | |
600 | === modified file 'tests/dbus/dbus_test.cpp' |
601 | --- tests/dbus/dbus_test.cpp 2015-06-08 03:59:53 +0000 |
602 | +++ tests/dbus/dbus_test.cpp 2015-06-17 06:15:32 +0000 |
603 | @@ -125,12 +125,8 @@ |
604 | TEST_F(DBusTest, thumbnail_image) |
605 | { |
606 | const char* filename = TESTDATADIR "/testimage.jpg"; |
607 | - FdPtr fd(open(filename, O_RDONLY), do_close); |
608 | - ASSERT_GE(fd.get(), 0); |
609 | - |
610 | QDBusReply<QDBusUnixFileDescriptor> reply = |
611 | - dbus_->thumbnailer_->GetThumbnail( |
612 | - filename, QDBusUnixFileDescriptor(fd.get()), QSize(256, 256)); |
613 | + dbus_->thumbnailer_->GetThumbnail(filename, QSize(256, 256)); |
614 | assert_no_error(reply); |
615 | |
616 | Image image(reply.value().fileDescriptor()); |
617 | @@ -144,12 +140,8 @@ |
618 | for (int i = 0; i < 2; ++i) |
619 | { |
620 | const char* filename = TESTDATADIR "/testsong.ogg"; |
621 | - FdPtr fd(open(filename, O_RDONLY), do_close); |
622 | - ASSERT_GE(fd.get(), 0); |
623 | - |
624 | QDBusReply<QDBusUnixFileDescriptor> reply = |
625 | - dbus_->thumbnailer_->GetThumbnail( |
626 | - filename, QDBusUnixFileDescriptor(fd.get()), QSize(256, 256)); |
627 | + dbus_->thumbnailer_->GetThumbnail(filename, QSize(256, 256)); |
628 | assert_no_error(reply); |
629 | |
630 | Image image(reply.value().fileDescriptor()); |
631 | @@ -164,12 +156,8 @@ |
632 | for (int i = 0; i < 2; ++i) |
633 | { |
634 | const char* filename = TESTDATADIR "/testvideo.ogg"; |
635 | - FdPtr fd(open(filename, O_RDONLY), do_close); |
636 | - ASSERT_GE(fd.get(), 0); |
637 | - |
638 | QDBusReply<QDBusUnixFileDescriptor> reply = |
639 | - dbus_->thumbnailer_->GetThumbnail( |
640 | - filename, QDBusUnixFileDescriptor(fd.get()), QSize(256, 256)); |
641 | + dbus_->thumbnailer_->GetThumbnail(filename, QSize(256, 256)); |
642 | assert_no_error(reply); |
643 | |
644 | Image image(reply.value().fileDescriptor()); |
645 | @@ -181,35 +169,13 @@ |
646 | TEST_F(DBusTest, thumbnail_no_such_file) |
647 | { |
648 | const char* no_such_file = TESTDATADIR "/no-such-file.jpg"; |
649 | - const char* filename2 = TESTDATADIR "/testrotate.jpg"; |
650 | - |
651 | - FdPtr fd(open(filename2, O_RDONLY), do_close); |
652 | - ASSERT_GE(fd.get(), 0); |
653 | - |
654 | QDBusReply<QDBusUnixFileDescriptor> reply = |
655 | - dbus_->thumbnailer_->GetThumbnail( |
656 | - no_such_file, QDBusUnixFileDescriptor(fd.get()), QSize(256, 256)); |
657 | + dbus_->thumbnailer_->GetThumbnail(no_such_file, QSize(256, 256)); |
658 | EXPECT_FALSE(reply.isValid()); |
659 | auto message = reply.error().message().toStdString(); |
660 | EXPECT_TRUE(boost::contains(message, " No such file or directory: ")) << message; |
661 | } |
662 | |
663 | -TEST_F(DBusTest, thumbnail_wrong_fd_fails) |
664 | -{ |
665 | - const char* filename1 = TESTDATADIR "/testimage.jpg"; |
666 | - const char* filename2 = TESTDATADIR "/testrotate.jpg"; |
667 | - |
668 | - FdPtr fd(open(filename2, O_RDONLY), do_close); |
669 | - ASSERT_GE(fd.get(), 0); |
670 | - |
671 | - QDBusReply<QDBusUnixFileDescriptor> reply = |
672 | - dbus_->thumbnailer_->GetThumbnail( |
673 | - filename1, QDBusUnixFileDescriptor(fd.get()), QSize(256, 256)); |
674 | - EXPECT_FALSE(reply.isValid()); |
675 | - auto message = reply.error().message().toStdString(); |
676 | - EXPECT_TRUE(boost::contains(message, " file descriptor does not refer to file ")) << message; |
677 | -} |
678 | - |
679 | TEST_F(DBusTest, duplicate_requests) |
680 | { |
681 | int const N_REQUESTS = 10; |
682 | @@ -264,16 +230,13 @@ |
683 | { |
684 | // basic setup to the query |
685 | const char* filename = TESTDATADIR "/testimage.jpg"; |
686 | - FdPtr fd(open(filename, O_RDONLY), do_close); |
687 | - ASSERT_GE(fd.get(), 0); |
688 | |
689 | QSignalSpy spy_exit(&dbus_->service_process(), |
690 | static_cast<void (QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished)); |
691 | |
692 | // start a query |
693 | QDBusReply<QDBusUnixFileDescriptor> reply = |
694 | - dbus_->thumbnailer_->GetThumbnail( |
695 | - filename, QDBusUnixFileDescriptor(fd.get()), QSize(256, 256)); |
696 | + dbus_->thumbnailer_->GetThumbnail(filename, QSize(256, 256)); |
697 | assert_no_error(reply); |
698 | |
699 | // wait for 5 seconds... (default) |
700 | |
701 | === modified file 'tests/qml/tst_albumart.qml' |
702 | --- tests/qml/tst_albumart.qml 2015-06-01 06:19:23 +0000 |
703 | +++ tests/qml/tst_albumart.qml 2015-06-17 06:15:32 +0000 |
704 | @@ -23,22 +23,14 @@ |
705 | |
706 | function test_artistart_not_found() { |
707 | loadArtistArt("beck2", "odelay") |
708 | - compare(size.width, 640); |
709 | - compare(size.height, 480); |
710 | - comparePixel(0, 0, "#FE0000"); |
711 | - comparePixel(639, 0, "#FFFF00"); |
712 | - comparePixel(0, 479, "#0000FE"); |
713 | - comparePixel(639, 479, "#00FF01"); |
714 | + compare(size.width, 96); |
715 | + compare(size.height, 96); |
716 | } |
717 | |
718 | function test_albumart_not_found() { |
719 | loadAlbumArt("beck2", "odelay") |
720 | - compare(size.width, 640); |
721 | - compare(size.height, 480); |
722 | - comparePixel(0, 0, "#FE0000"); |
723 | - comparePixel(639, 0, "#FFFF00"); |
724 | - comparePixel(0, 479, "#0000FE"); |
725 | - comparePixel(639, 479, "#00FF01"); |
726 | + compare(size.width, 96); |
727 | + compare(size.height, 96); |
728 | } |
729 | |
730 | function test_bad_artist_art_url() { |
731 | |
732 | === modified file 'tests/testsetup.h.in' |
733 | --- tests/testsetup.h.in 2015-06-01 04:12:08 +0000 |
734 | +++ tests/testsetup.h.in 2015-06-17 06:15:32 +0000 |
735 | @@ -27,6 +27,6 @@ |
736 | |
737 | #define THUMBNAILER_SERVICE "@CMAKE_BINARY_DIR@/src/service/thumbnailer-service" |
738 | #define FAKE_ART_SERVER "@CMAKE_CURRENT_SOURCE_DIR@/server/server.py" |
739 | -#define THUMBNAILER_TEST_DEFAULT_IMAGE "@CMAKE_CURRENT_SOURCE_DIR@/media/orientation-8.jpg" |
740 | +#define THUMBNAILER_TEST_DEFAULT_IMAGE "@CMAKE_SOURCE_DIR@/data/album_missing.png" |
741 | |
742 | #define THUMBNAILER_ADMIN "@CMAKE_BINARY_DIR@/src/thumbnailer-admin/thumbnailer-admin" |
743 | |
744 | === modified file 'tests/thumbnailer-admin/thumbnailer-admin_test.cpp' |
745 | --- tests/thumbnailer-admin/thumbnailer-admin_test.cpp 2015-06-08 03:25:50 +0000 |
746 | +++ tests/thumbnailer-admin/thumbnailer-admin_test.cpp 2015-06-17 06:15:32 +0000 |
747 | @@ -471,8 +471,8 @@ |
748 | AdminRunner ar; |
749 | |
750 | EXPECT_EQ(1, ar.run(QStringList{"get", "no_such_file"})); |
751 | - EXPECT_EQ("thumbnailer-admin: GetLocalThumbnail::run(): cannot open no_such_file: No such file or directory\n", |
752 | - ar.stderr()) |
753 | + EXPECT_TRUE(starts_with(ar.stderr(), |
754 | + "thumbnailer-admin: DBusInterface::GetThumbnail(): no_such_file: unity::ResourceException: Thumbnailer::get_thumbnail():\n boost::filesystem::canonical: No such file or directory:")) |
755 | << ar.stderr(); |
756 | |
757 | EXPECT_EQ(1, ar.run(QStringList{"get", TESTDATADIR "/orientation-2.jpg", "no_such_directory"})); |
758 | |
759 | === modified file 'tests/thumbnailer/thumbnailer_test.cpp' |
760 | --- tests/thumbnailer/thumbnailer_test.cpp 2015-06-10 12:30:59 +0000 |
761 | +++ tests/thumbnailer/thumbnailer_test.cpp 2015-06-17 06:15:32 +0000 |
762 | @@ -91,19 +91,20 @@ |
763 | std::unique_ptr<ThumbnailRequest> request; |
764 | string thumb; |
765 | Image img; |
766 | - FdPtr fd(-1, do_close); |
767 | |
768 | - fd.reset(open(EMPTY_IMAGE, O_RDONLY)); |
769 | - thumb = tn.get_thumbnail(EMPTY_IMAGE, fd.get(), QSize())->thumbnail(); |
770 | + request = tn.get_thumbnail(EMPTY_IMAGE, QSize()); |
771 | + request->set_client_credentials(geteuid(), "unconfined"); |
772 | + thumb = request->thumbnail(); |
773 | EXPECT_EQ("", thumb); |
774 | |
775 | // Again, this time we get the answer from the failure cache. |
776 | - fd.reset(open(EMPTY_IMAGE, O_RDONLY)); |
777 | - thumb = tn.get_thumbnail(EMPTY_IMAGE, fd.get(), QSize())->thumbnail(); |
778 | + request = tn.get_thumbnail(EMPTY_IMAGE, QSize()); |
779 | + request->set_client_credentials(geteuid(), "unconfined"); |
780 | + thumb = request->thumbnail(); |
781 | EXPECT_EQ("", thumb); |
782 | |
783 | - fd.reset(open(TEST_IMAGE, O_RDONLY)); |
784 | - request = tn.get_thumbnail(TEST_IMAGE, fd.get(), QSize()); |
785 | + request = tn.get_thumbnail(TEST_IMAGE, QSize()); |
786 | + request->set_client_credentials(geteuid(), "unconfined"); |
787 | EXPECT_TRUE(boost::starts_with(request->key(), TEST_IMAGE)) << request->key(); |
788 | thumb = request->thumbnail(); |
789 | img = Image(thumb); |
790 | @@ -111,30 +112,40 @@ |
791 | EXPECT_EQ(480, img.height()); |
792 | |
793 | // Again, for coverage. This time the thumbnail comes from the cache. |
794 | - thumb = tn.get_thumbnail(TEST_IMAGE, fd.get(), QSize())->thumbnail(); |
795 | + request = tn.get_thumbnail(TEST_IMAGE, QSize()); |
796 | + request->set_client_credentials(geteuid(), "unconfined"); |
797 | + thumb = request->thumbnail(); |
798 | img = Image(thumb); |
799 | EXPECT_EQ(640, img.width()); |
800 | EXPECT_EQ(480, img.height()); |
801 | |
802 | - thumb = tn.get_thumbnail(TEST_IMAGE, fd.get(), QSize(160, 160))->thumbnail(); |
803 | + request = tn.get_thumbnail(TEST_IMAGE, QSize(160, 160)); |
804 | + request->set_client_credentials(geteuid(), "unconfined"); |
805 | + thumb = request->thumbnail(); |
806 | img = Image(thumb); |
807 | EXPECT_EQ(160, img.width()); |
808 | EXPECT_EQ(120, img.height()); |
809 | |
810 | - thumb = tn.get_thumbnail(TEST_IMAGE, fd.get(), QSize(1000, 1000))->thumbnail(); // Will not up-scale |
811 | + request = tn.get_thumbnail(TEST_IMAGE, QSize(1000, 1000)); // Will not up-scale |
812 | + request->set_client_credentials(geteuid(), "unconfined"); |
813 | + thumb = request->thumbnail(); |
814 | img = Image(thumb); |
815 | EXPECT_EQ(640, img.width()); |
816 | EXPECT_EQ(480, img.height()); |
817 | |
818 | - thumb = tn.get_thumbnail(TEST_IMAGE, fd.get(), QSize(100, 100))->thumbnail(); // From EXIF data |
819 | + request = tn.get_thumbnail(TEST_IMAGE, QSize(100, 100)); // From EXIF data |
820 | + request->set_client_credentials(geteuid(), "unconfined"); |
821 | + thumb = request->thumbnail(); |
822 | img = Image(thumb); |
823 | EXPECT_EQ(100, img.width()); |
824 | EXPECT_EQ(75, img.height()); |
825 | |
826 | - fd.reset(open(BAD_IMAGE, O_RDONLY)); |
827 | try |
828 | { |
829 | - tn.get_thumbnail(BAD_IMAGE, fd.get(), QSize())->thumbnail(); |
830 | + request = tn.get_thumbnail(BAD_IMAGE, QSize()); |
831 | + request->set_client_credentials(geteuid(), "unconfined"); |
832 | + request->thumbnail(); |
833 | + FAIL(); |
834 | } |
835 | catch (std::exception const& e) |
836 | { |
837 | @@ -142,20 +153,23 @@ |
838 | EXPECT_TRUE(boost::starts_with(msg, "unity::ResourceException: RequestBase::thumbnail(): key = ")) << msg; |
839 | } |
840 | |
841 | - fd.reset(open(RGB_IMAGE, O_RDONLY)); |
842 | - thumb = tn.get_thumbnail(RGB_IMAGE, fd.get(), QSize(48, 48))->thumbnail(); |
843 | + request = tn.get_thumbnail(RGB_IMAGE, QSize(48, 48)); |
844 | + request->set_client_credentials(geteuid(), "unconfined"); |
845 | + thumb = request->thumbnail(); |
846 | img = Image(thumb); |
847 | EXPECT_EQ(48, img.width()); |
848 | EXPECT_EQ(48, img.height()); |
849 | |
850 | - fd.reset(open(BIG_IMAGE, O_RDONLY)); |
851 | - thumb = tn.get_thumbnail(BIG_IMAGE, fd.get(), QSize())->thumbnail(); // > 1920, so will be trimmed down |
852 | + request = tn.get_thumbnail(BIG_IMAGE, QSize()); // > 1920, so will be trimmed down |
853 | + request->set_client_credentials(geteuid(), "unconfined"); |
854 | + thumb = request->thumbnail(); |
855 | img = Image(thumb); |
856 | EXPECT_EQ(1920, img.width()); |
857 | EXPECT_EQ(1439, img.height()); |
858 | |
859 | - thumb = |
860 | - tn.get_thumbnail(BIG_IMAGE, fd.get(), QSize(0, 0))->thumbnail(); // unconstrained, so will not be trimmed down |
861 | + request = tn.get_thumbnail(BIG_IMAGE, QSize(0, 0)); // unconstrained, so will not be trimmed down |
862 | + request->set_client_credentials(geteuid(), "unconfined"); |
863 | + thumb = request->thumbnail(); |
864 | img = Image(thumb); |
865 | EXPECT_EQ(2731, img.width()); |
866 | EXPECT_EQ(2048, img.height()); |
867 | @@ -184,9 +198,9 @@ |
868 | { |
869 | { |
870 | // Load a song so we have something in the full-size and thumbnail caches. |
871 | - FdPtr fd(open(TEST_SONG, O_RDONLY), do_close); |
872 | - auto request = tn.get_thumbnail(TEST_SONG, fd.get(), QSize()); |
873 | + auto request = tn.get_thumbnail(TEST_SONG, QSize()); |
874 | ASSERT_NE(nullptr, request.get()); |
875 | + request->set_client_credentials(geteuid(), "unconfined"); |
876 | // Audio thumbnails cannot be produced immediately |
877 | ASSERT_EQ("", request->thumbnail()); |
878 | |
879 | @@ -202,32 +216,32 @@ |
880 | |
881 | { |
882 | // Load same song again at different size, so we get a hit on full-size cache. |
883 | - FdPtr fd(open(TEST_SONG, O_RDONLY), do_close); |
884 | - auto request = tn.get_thumbnail(TEST_SONG, fd.get(), QSize(20, 20)); |
885 | + auto request = tn.get_thumbnail(TEST_SONG, QSize(20, 20)); |
886 | ASSERT_NE(nullptr, request.get()); |
887 | + request->set_client_credentials(geteuid(), "unconfined"); |
888 | ASSERT_NE("", request->thumbnail()); |
889 | } |
890 | |
891 | { |
892 | // Load same song again at same size, so we get a hit on thumbnail cache. |
893 | - FdPtr fd(open(TEST_SONG, O_RDONLY), do_close); |
894 | - auto request = tn.get_thumbnail(TEST_SONG, fd.get(), QSize(20, 20)); |
895 | + auto request = tn.get_thumbnail(TEST_SONG, QSize(20, 20)); |
896 | ASSERT_NE(nullptr, request.get()); |
897 | + request->set_client_credentials(geteuid(), "unconfined"); |
898 | ASSERT_NE("", request->thumbnail()); |
899 | } |
900 | |
901 | { |
902 | // Load an empty image, so we have something in the failure cache. |
903 | - FdPtr fd(open(EMPTY_IMAGE, O_RDONLY), do_close); |
904 | - string thumb = tn.get_thumbnail(EMPTY_IMAGE, fd.get(), QSize())->thumbnail(); |
905 | - EXPECT_EQ("", thumb); |
906 | + auto request = tn.get_thumbnail(EMPTY_IMAGE, QSize()); |
907 | + request->set_client_credentials(geteuid(), "unconfined"); |
908 | + EXPECT_EQ("", request->thumbnail()); |
909 | } |
910 | |
911 | { |
912 | // Load empty image again, so we get a hit on failure cache. |
913 | - FdPtr fd(open(EMPTY_IMAGE, O_RDONLY), do_close); |
914 | - string thumb = tn.get_thumbnail(EMPTY_IMAGE, fd.get(), QSize())->thumbnail(); |
915 | - EXPECT_EQ("", thumb); |
916 | + auto request = tn.get_thumbnail(EMPTY_IMAGE, QSize()); |
917 | + request->set_client_credentials(geteuid(), "unconfined"); |
918 | + EXPECT_EQ("", request->thumbnail()); |
919 | } |
920 | }; |
921 | |
922 | @@ -313,50 +327,20 @@ |
923 | EXPECT_EQ(0, stats.failure_stats.hits()); |
924 | } |
925 | |
926 | -TEST_F(ThumbnailerTest, bad_fd) |
927 | -{ |
928 | - Thumbnailer tn; |
929 | - |
930 | - // Invalid file descriptor |
931 | - try |
932 | - { |
933 | - tn.get_thumbnail(TEST_IMAGE, -1, QSize()); |
934 | - } |
935 | - catch (std::exception const& e) |
936 | - { |
937 | - string msg = e.what(); |
938 | - EXPECT_TRUE(boost::contains(msg, ": Could not stat file descriptor:")) << msg; |
939 | - } |
940 | - |
941 | - // File descriptor for wrong file |
942 | - FdPtr fd(open(TEST_VIDEO, O_RDONLY), do_close); |
943 | - try |
944 | - { |
945 | - tn.get_thumbnail(TEST_IMAGE, fd.get(), QSize()); |
946 | - } |
947 | - catch (std::exception const& e) |
948 | - { |
949 | - string msg = e.what(); |
950 | - EXPECT_TRUE(boost::contains(msg, ": file descriptor does not refer to file ")) << msg; |
951 | - } |
952 | -} |
953 | - |
954 | -TEST_F(ThumbnailerTest, replace_photo) |
955 | +TEST_F(ThumbnailerTest, DISABLED_replace_photo) |
956 | { |
957 | string testfile = tempdir_path() + "/foo.jpg"; |
958 | ASSERT_EQ(0, link(TEST_IMAGE, testfile.c_str())); |
959 | |
960 | Thumbnailer tn; |
961 | - FdPtr fd(open(testfile.c_str(), O_RDONLY), do_close); |
962 | - auto request = tn.get_thumbnail(testfile, fd.get(), QSize()); |
963 | - // The client FD isn't needed any more, so close it. |
964 | - fd.reset(-1); |
965 | + auto request = tn.get_thumbnail(testfile, QSize()); |
966 | |
967 | // Replace test image with a different file with different |
968 | // dimensions so we can tell which one is thumbnailed. |
969 | ASSERT_EQ(0, unlink(testfile.c_str())); |
970 | ASSERT_EQ(0, link(BIG_IMAGE, testfile.c_str())); |
971 | |
972 | + request->set_client_credentials(geteuid(), "unconfined"); |
973 | string data = request->thumbnail(); |
974 | Image img(data); |
975 | EXPECT_EQ(640, img.width()); |
976 | @@ -366,9 +350,9 @@ |
977 | TEST_F(ThumbnailerTest, thumbnail_video) |
978 | { |
979 | Thumbnailer tn; |
980 | - FdPtr fd(open(TEST_VIDEO, O_RDONLY), do_close); |
981 | - auto request = tn.get_thumbnail(TEST_VIDEO, fd.get(), QSize()); |
982 | + auto request = tn.get_thumbnail(TEST_VIDEO, QSize()); |
983 | ASSERT_NE(nullptr, request.get()); |
984 | + request->set_client_credentials(geteuid(), "unconfined"); |
985 | // Video thumbnails cannot be produced immediately |
986 | ASSERT_EQ("", request->thumbnail()); |
987 | |
988 | @@ -386,7 +370,8 @@ |
989 | { |
990 | // Fetch the thumbnail again with the same size. |
991 | // That causes it to come from the thumbnail cache. |
992 | - auto request = tn.get_thumbnail(TEST_VIDEO, fd.get(), QSize()); |
993 | + auto request = tn.get_thumbnail(TEST_VIDEO, QSize()); |
994 | + request->set_client_credentials(geteuid(), "unconfined"); |
995 | string thumb = request->thumbnail(); |
996 | ASSERT_NE("", thumb); |
997 | Image img(thumb); |
998 | @@ -397,7 +382,8 @@ |
999 | { |
1000 | // Fetch the thumbnail again with a different size. |
1001 | // That causes it to be scaled from the thumbnail cache. |
1002 | - auto request = tn.get_thumbnail(TEST_VIDEO, fd.get(), QSize(500, 500)); |
1003 | + auto request = tn.get_thumbnail(TEST_VIDEO, QSize(500, 500)); |
1004 | + request->set_client_credentials(geteuid(), "unconfined"); |
1005 | string thumb = request->thumbnail(); |
1006 | ASSERT_NE("", thumb); |
1007 | Image img(thumb); |
1008 | @@ -412,17 +398,15 @@ |
1009 | ASSERT_EQ(0, link(TEST_VIDEO, testfile.c_str())) << strerror(errno); |
1010 | |
1011 | Thumbnailer tn; |
1012 | - FdPtr fd(open(testfile.c_str(), O_RDONLY | O_CLOEXEC), do_close); |
1013 | - auto request = tn.get_thumbnail(testfile, fd.get(), QSize()); |
1014 | - // The client FD isn't needed any more, so close it. |
1015 | - fd.reset(-1); |
1016 | + auto request = tn.get_thumbnail(testfile, QSize()); |
1017 | + request->set_client_credentials(geteuid(), "unconfined"); |
1018 | + ASSERT_EQ("", request->thumbnail()); |
1019 | |
1020 | // Replace test image with a different file with different |
1021 | // dimensions so we can tell which one is thumbnailed. |
1022 | ASSERT_EQ(0, unlink(testfile.c_str())); |
1023 | ASSERT_EQ(0, link(BIG_IMAGE, testfile.c_str())); |
1024 | |
1025 | - ASSERT_EQ("", request->thumbnail()); |
1026 | QSignalSpy spy(request.get(), &ThumbnailRequest::downloadFinished); |
1027 | request->download(chrono::milliseconds(15000)); |
1028 | ASSERT_TRUE(spy.wait(20000)); |
1029 | @@ -436,9 +420,9 @@ |
1030 | TEST_F(ThumbnailerTest, thumbnail_song) |
1031 | { |
1032 | Thumbnailer tn; |
1033 | - FdPtr fd(open(TEST_SONG, O_RDONLY), do_close); |
1034 | - auto request = tn.get_thumbnail(TEST_SONG, fd.get(), QSize()); |
1035 | + auto request = tn.get_thumbnail(TEST_SONG, QSize()); |
1036 | ASSERT_NE(nullptr, request.get()); |
1037 | + request->set_client_credentials(geteuid(), "unconfined"); |
1038 | // Audio thumbnails cannot be produced immediately |
1039 | ASSERT_EQ("", request->thumbnail()); |
1040 | |
1041 | @@ -482,8 +466,8 @@ |
1042 | |
1043 | setenv("TN_UTILDIR", "no_such_directory", true); |
1044 | |
1045 | - FdPtr fd(open(TEST_SONG, O_RDONLY), do_close); |
1046 | - auto request = tn.get_thumbnail(TEST_SONG, fd.get(), QSize()); |
1047 | + auto request = tn.get_thumbnail(TEST_SONG, QSize()); |
1048 | + request->set_client_credentials(geteuid(), "unconfined"); |
1049 | EXPECT_EQ("", request->thumbnail()); |
1050 | |
1051 | QSignalSpy spy(request.get(), &ThumbnailRequest::downloadFinished); |
1052 | @@ -615,7 +599,7 @@ |
1053 | |
1054 | try |
1055 | { |
1056 | - auto request = tn.get_thumbnail("no_such_file", -1, QSize()); |
1057 | + auto request = tn.get_thumbnail("no_such_file", QSize()); |
1058 | FAIL(); |
1059 | } |
1060 | catch (unity::ResourceException const& e) |
FAILED: Continuous integration, rev:222 /code.launchpad .net/~jamesh/ thumbnailer/ use-aa- query-label/ +merge/ 262060/ +edit-commit- message
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https:/
http:// jenkins. qa.ubuntu. com/job/ thumbnailer- devel-ci/ 285/ jenkins. qa.ubuntu. com/job/ thumbnailer- devel-wily- amd64-ci/ 94/console jenkins. qa.ubuntu. com/job/ thumbnailer- devel-wily- armhf-ci/ 96/console jenkins. qa.ubuntu. com/job/ thumbnailer- devel-wily- i386-ci/ 94/console
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/thumbnailer -devel- ci/285/ rebuild
http://