Merge lp:~phablet-team/media-hub/fix-1449790 into lp:media-hub

Proposed by Jim Hodapp
Status: Merged
Approved by: Alfonso Sanchez-Beato
Approved revision: 170
Merged at revision: 170
Proposed branch: lp:~phablet-team/media-hub/fix-1449790
Merge into: lp:media-hub
Diff against target: 780 lines (+429/-67)
13 files modified
include/core/media/player.h (+5/-0)
src/core/media/gstreamer/engine.cpp (+6/-3)
src/core/media/gstreamer/playbin.cpp (+129/-28)
src/core/media/gstreamer/playbin.h (+3/-0)
src/core/media/mpris/player.h (+8/-0)
src/core/media/player.cpp (+6/-0)
src/core/media/player_implementation.cpp (+2/-2)
src/core/media/player_skeleton.cpp (+57/-22)
src/core/media/player_stub.cpp (+2/-0)
src/core/media/track_list_implementation.cpp (+0/-1)
src/core/media/track_list_skeleton.cpp (+46/-11)
src/core/media/track_list_stub.cpp (+5/-0)
src/core/media/util/uri_check.h (+160/-0)
To merge this branch: bzr merge lp:~phablet-team/media-hub/fix-1449790
Reviewer Review Type Date Requested Status
Alfonso Sanchez-Beato Approve
PS Jenkins bot continuous-integration Approve
Review via email: mp+285813@code.launchpad.net

Commit message

Make sure to interpret the incoming path as a URI which makes sure media-hub will always be able to open the track with an encoded URI

Description of the change

Make sure to interpret the incoming path as a URI which makes sure media-hub will always be able to open the track with an encoded URI

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

LGTM

review: Approve
171. By Jim Hodapp

Do the percent encoding and checking only from media-hub and don't involve qtubuntu-media

172. By Jim Hodapp

Get rid of some debug statements

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

See comments below.

review: Needs Fixing
Revision history for this message
Jim Hodapp (jhodapp) :
173. By Jim Hodapp

Make sure other tracks can still be added to the tracklist after an invalid URI is determined. Also make sure to free all gchar strings

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

Still some comments, see below.

review: Needs Fixing
174. By Jim Hodapp

Address review comments

175. By Jim Hodapp

One last comment addressed

176. By Jim Hodapp

Make sure to not crash during std::string::assign(gchar*)

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

LGTM

review: Approve
177. By Jim Hodapp

Removed no longer needed debug statement

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'include/core/media/player.h'
--- include/core/media/player.h 2016-01-14 19:47:40 +0000
+++ include/core/media/player.h 2016-02-22 18:59:27 +0000
@@ -61,6 +61,11 @@
61 {61 {
62 InsufficientAppArmorPermissions(const std::string& err);62 InsufficientAppArmorPermissions(const std::string& err);
63 };63 };
64
65 struct UriNotFound : public std::runtime_error
66 {
67 UriNotFound(const std::string& err);
68 };
64 };69 };
6570
66 struct Configuration;71 struct Configuration;
6772
=== modified file 'src/core/media/gstreamer/engine.cpp'
--- src/core/media/gstreamer/engine.cpp 2015-09-04 18:46:08 +0000
+++ src/core/media/gstreamer/engine.cpp 2016-02-22 18:59:27 +0000
@@ -400,7 +400,8 @@
400 d->state = media::Engine::State::no_media;400 d->state = media::Engine::State::no_media;
401}401}
402402
403const std::shared_ptr<media::Engine::MetaDataExtractor>& gstreamer::Engine::meta_data_extractor() const403const std::shared_ptr<media::Engine::MetaDataExtractor>&
404 gstreamer::Engine::meta_data_extractor() const
404{405{
405 return d->meta_data_extractor;406 return d->meta_data_extractor;
406}407}
@@ -410,13 +411,15 @@
410 return d->state;411 return d->state;
411}412}
412413
413bool gstreamer::Engine::open_resource_for_uri(const media::Track::UriType& uri, bool do_pipeline_reset)414bool gstreamer::Engine::open_resource_for_uri(const media::Track::UriType& uri,
415 bool do_pipeline_reset)
414{416{
415 d->playbin.set_uri(uri, core::ubuntu::media::Player::HeadersType{}, do_pipeline_reset);417 d->playbin.set_uri(uri, core::ubuntu::media::Player::HeadersType{}, do_pipeline_reset);
416 return true;418 return true;
417}419}
418420
419bool gstreamer::Engine::open_resource_for_uri(const media::Track::UriType& uri, const core::ubuntu::media::Player::HeadersType& headers)421bool gstreamer::Engine::open_resource_for_uri(const media::Track::UriType& uri,
422 const core::ubuntu::media::Player::HeadersType& headers)
420{423{
421 d->playbin.set_uri(uri, headers);424 d->playbin.set_uri(uri, headers);
422 return true;425 return true;
423426
=== modified file 'src/core/media/gstreamer/playbin.cpp'
--- src/core/media/gstreamer/playbin.cpp 2015-12-16 16:55:51 +0000
+++ src/core/media/gstreamer/playbin.cpp 2016-02-22 18:59:27 +0000
@@ -17,7 +17,6 @@
17 */17 */
1818
19#include <core/media/gstreamer/playbin.h>19#include <core/media/gstreamer/playbin.h>
20
21#include <core/media/gstreamer/engine.h>20#include <core/media/gstreamer/engine.h>
2221
23#include <gst/pbutils/missing-plugins.h>22#include <gst/pbutils/missing-plugins.h>
@@ -26,8 +25,12 @@
26#include <hybris/media/surface_texture_client_hybris.h>25#include <hybris/media/surface_texture_client_hybris.h>
27#include <hybris/media/media_codec_layer.h>26#include <hybris/media/media_codec_layer.h>
2827
28#include "../util/uri_check.h"
29
29#include <utility>30#include <utility>
3031
32//#define VERBOSE_DEBUG
33
31namespace34namespace
32{35{
33void setup_video_sink_for_buffer_streaming(GstElement* pipeline)36void setup_video_sink_for_buffer_streaming(GstElement* pipeline)
@@ -464,10 +467,23 @@
464 if (current_uri and do_pipeline_reset)467 if (current_uri and do_pipeline_reset)
465 reset_pipeline();468 reset_pipeline();
466469
467 g_object_set(pipeline, "uri", uri.c_str(), NULL);470 std::string encoded_uri;
468 if (is_video_file(uri))471 media::UriCheck::Ptr uri_check{std::make_shared<media::UriCheck>(uri)};
472 if (uri_check->is_encoded())
473 {
474 encoded_uri = decode_uri(uri);
475#ifdef VERBOSE_DEBUG
476 std::cout << "File URI was encoded, now decoded: " << encoded_uri << std::endl;
477#endif
478 encoded_uri = encode_uri(encoded_uri);
479 }
480 else
481 encoded_uri = encode_uri(uri);
482
483 g_object_set(pipeline, "uri", encoded_uri.c_str(), NULL);
484 if (is_video_file(encoded_uri))
469 file_type = MEDIA_FILE_TYPE_VIDEO;485 file_type = MEDIA_FILE_TYPE_VIDEO;
470 else if (is_audio_file(uri))486 else if (is_audio_file(encoded_uri))
471 file_type = MEDIA_FILE_TYPE_AUDIO;487 file_type = MEDIA_FILE_TYPE_AUDIO;
472488
473 request_headers = headers;489 request_headers = headers;
@@ -607,25 +623,13 @@
607 signals.on_video_dimensions_changed(new_dimensions);623 signals.on_video_dimensions_changed(new_dimensions);
608}624}
609625
610std::string gstreamer::Playbin::get_file_content_type(const std::string& uri) const626std::string gstreamer::Playbin::file_info_from_uri(const std::string& uri) const
611{627{
612 if (uri.empty())
613 return std::string();
614
615 std::string filename(uri);
616 size_t pos = uri.find("file://");
617 if (pos != std::string::npos)
618 filename = uri.substr(pos + 7, std::string::npos);
619 else
620 // Anything other than a file, for now claim that the type
621 // is both audio and video.
622 // FIXME: implement true net stream sampling and get the type from GstCaps
623 return std::string("audio/video/");
624
625
626 GError *error = nullptr;628 GError *error = nullptr;
629 // Open the URI and get the mime type from it. This will currently only work for
630 // a local file
627 std::unique_ptr<GFile, void(*)(void *)> file(631 std::unique_ptr<GFile, void(*)(void *)> file(
628 g_file_new_for_path(filename.c_str()), g_object_unref);632 g_file_new_for_uri(uri.c_str()), g_object_unref);
629 std::unique_ptr<GFileInfo, void(*)(void *)> info(633 std::unique_ptr<GFileInfo, void(*)(void *)> info(
630 g_file_query_info(634 g_file_query_info(
631 file.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE ","635 file.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE ","
@@ -633,17 +637,114 @@
633 /* cancellable */ NULL, &error),637 /* cancellable */ NULL, &error),
634 g_object_unref);638 g_object_unref);
635 if (!info)639 if (!info)
636 {
637 std::string error_str(error->message);
638 g_error_free(error);
639
640 std::cout << "Failed to query the URI for the presence of video content: "
641 << error_str << std::endl;
642 return std::string();640 return std::string();
643 }
644641
645 std::string content_type(g_file_info_get_attribute_string(642 return std::string(g_file_info_get_attribute_string(
646 info.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE));643 info.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE));
644}
645
646std::string gstreamer::Playbin::encode_uri(const std::string& uri) const
647{
648 if (uri.empty())
649 return std::string();
650
651 std::string encoded_uri;
652 media::UriCheck::Ptr uri_check{std::make_shared<media::UriCheck>(uri)};
653 gchar *uri_scheme = g_uri_parse_scheme(uri.c_str());
654 // We have a URI and it is already percent encoded
655 if (uri_scheme and strlen(uri_scheme) > 0 and uri_check->is_encoded())
656 {
657#ifdef VERBOSE_DEBUG
658 std::cout << "Is a URI and is already percent encoded" << std::endl;
659#endif
660 encoded_uri = uri;
661 }
662 // We have a URI but it's not already percent encoded
663 else if (uri_scheme and strlen(uri_scheme) > 0 and !uri_check->is_encoded())
664 {
665#ifdef VERBOSE_DEBUG
666 std::cout << "Is a URI and is not already percent encoded" << std::endl;
667#endif
668 gchar *encoded = g_uri_escape_string(uri.c_str(),
669 "!$&'()*+,;=:/?[]@", // reserved chars
670 TRUE); // Allow UTF-8 chars
671 if (!encoded)
672 {
673 g_free(uri_scheme);
674 return std::string();
675 }
676 encoded_uri.assign(encoded);
677 g_free(encoded);
678 }
679 else // We have a path and not a URI. Turn it into a full URI and encode it
680 {
681 GError *error = nullptr;
682#ifdef VERBOSE_DEBUG
683 std::cout << "Is a path and is not already percent encoded" << std::endl;
684#endif
685 gchar *str = g_filename_to_uri(uri.c_str(), nullptr, &error);
686 if (!str)
687 {
688 g_free(uri_scheme);
689 return std::string();
690 }
691 encoded_uri.assign(str);
692 g_free(str);
693 if (error != nullptr)
694 {
695 std::cerr << "Warning: failed to get actual track content type: " << error->message
696 << std::endl;
697 g_error_free(error);
698 g_free(str);
699 g_free(uri_scheme);
700 return std::string("audio/video/");
701 }
702 gchar *escaped = g_uri_escape_string(encoded_uri.c_str(),
703 "!$&'()*+,;=:/?[]@", // reserved chars
704 TRUE); // Allow UTF-8 chars
705 if (!escaped)
706 {
707 g_free(uri_scheme);
708 return std::string();
709 }
710 encoded_uri.assign(escaped);
711 g_free(escaped);
712 }
713
714 g_free(uri_scheme);
715
716 return encoded_uri;
717}
718
719std::string gstreamer::Playbin::decode_uri(const std::string& uri) const
720{
721 if (uri.empty())
722 return std::string();
723
724 gchar *decoded_gchar = g_uri_unescape_string(uri.c_str(), nullptr);
725 if (!decoded_gchar)
726 return std::string();
727
728 const std::string decoded{decoded_gchar};
729 g_free(decoded_gchar);
730 return decoded;
731}
732
733std::string gstreamer::Playbin::get_file_content_type(const std::string& uri) const
734{
735 if (uri.empty())
736 return std::string();
737
738 const std::string encoded_uri{encode_uri(uri)};
739
740 const std::string content_type {file_info_from_uri(encoded_uri)};
741 if (content_type.empty())
742 {
743 std::cerr << "Warning: failed to get actual track content type" << std::endl;
744 return std::string("audio/video/");
745 }
746
747 std::cout << "Found content type: " << content_type << std::endl;
647748
648 return content_type;749 return content_type;
649}750}
650751
=== modified file 'src/core/media/gstreamer/playbin.h'
--- src/core/media/gstreamer/playbin.h 2015-09-23 11:17:26 +0000
+++ src/core/media/gstreamer/playbin.h 2016-02-22 18:59:27 +0000
@@ -104,6 +104,9 @@
104 core::ubuntu::media::video::Dimensions get_video_dimensions() const;104 core::ubuntu::media::video::Dimensions get_video_dimensions() const;
105 void emit_video_dimensions_changed_if_changed(const core::ubuntu::media::video::Dimensions &new_dimensions);105 void emit_video_dimensions_changed_if_changed(const core::ubuntu::media::video::Dimensions &new_dimensions);
106106
107 std::string file_info_from_uri(const std::string& uri) const;
108 std::string encode_uri(const std::string& uri) const;
109 std::string decode_uri(const std::string& uri) const;
107 std::string get_file_content_type(const std::string& uri) const;110 std::string get_file_content_type(const std::string& uri) const;
108111
109 bool is_audio_file(const std::string& uri) const;112 bool is_audio_file(const std::string& uri) const;
110113
=== modified file 'src/core/media/mpris/player.h'
--- src/core/media/mpris/player.h 2016-01-14 19:47:40 +0000
+++ src/core/media/mpris/player.h 2016-02-22 18:59:27 +0000
@@ -124,6 +124,14 @@
124 "mpris.Player.Error.InsufficientAppArmorPermissions"124 "mpris.Player.Error.InsufficientAppArmorPermissions"
125 };125 };
126 };126 };
127
128 struct UriNotFound
129 {
130 static constexpr const char* name
131 {
132 "mpris.Player.Error.UriNotFound"
133 };
134 };
127 };135 };
128136
129 typedef std::map<std::string, core::dbus::types::Variant> Dictionary;137 typedef std::map<std::string, core::dbus::types::Variant> Dictionary;
130138
=== modified file 'src/core/media/player.cpp'
--- src/core/media/player.cpp 2016-01-14 19:47:40 +0000
+++ src/core/media/player.cpp 2016-02-22 18:59:27 +0000
@@ -33,6 +33,12 @@
33{33{
34}34}
3535
36media::Player::Errors::UriNotFound::UriNotFound
37 (const std::string &e)
38 : std::runtime_error{e}
39{
40}
41
36const media::Player::Configuration& media::Player::Client::default_configuration()42const media::Player::Configuration& media::Player::Client::default_configuration()
37{43{
38 static const media::Player::Configuration config44 static const media::Player::Configuration config
3945
=== modified file 'src/core/media/player_implementation.cpp'
--- src/core/media/player_implementation.cpp 2015-11-18 15:55:23 +0000
+++ src/core/media/player_implementation.cpp 2016-02-22 18:59:27 +0000
@@ -292,8 +292,8 @@
292 const Track::UriType uri = track_list->query_uri_for_track(id);292 const Track::UriType uri = track_list->query_uri_for_track(id);
293 if (!uri.empty())293 if (!uri.empty())
294 {294 {
295 // Using a TrackList for playback, added tracks via add_track(), but open_uri hasn't been called yet295 // Using a TrackList for playback, added tracks via add_track(), but open_uri hasn't
296 // to load a media resource296 // been called yet to load a media resource
297 std::cout << "Calling d->engine->open_resource_for_uri() for first track added only: "297 std::cout << "Calling d->engine->open_resource_for_uri() for first track added only: "
298 << uri << std::endl;298 << uri << std::endl;
299 std::cout << "\twith a Track::Id: " << id << std::endl;299 std::cout << "\twith a Track::Id: " << id << std::endl;
300300
=== modified file 'src/core/media/player_skeleton.cpp'
--- src/core/media/player_skeleton.cpp 2016-01-14 20:08:38 +0000
+++ src/core/media/player_skeleton.cpp 2016-02-22 18:59:27 +0000
@@ -31,6 +31,7 @@
31#include "mpris/metadata.h"31#include "mpris/metadata.h"
32#include "mpris/player.h"32#include "mpris/player.h"
33#include "mpris/playlists.h"33#include "mpris/playlists.h"
34#include "util/uri_check.h"
3435
35#include <core/dbus/object.h>36#include <core/dbus/object.h>
36#include <core/dbus/property.h>37#include <core/dbus/property.h>
@@ -54,6 +55,7 @@
54 object(session),55 object(session),
55 request_context_resolver{request_context_resolver},56 request_context_resolver{request_context_resolver},
56 request_authenticator{request_authenticator},57 request_authenticator{request_authenticator},
58 uri_check(std::make_shared<UriCheck>()),
57 skeleton{mpris::Player::Skeleton::Configuration{bus, session, mpris::Player::Skeleton::Configuration::Defaults{}}},59 skeleton{mpris::Player::Skeleton::Configuration{bus, session, mpris::Player::Skeleton::Configuration::Defaults{}}},
58 signals60 signals
59 {61 {
@@ -180,22 +182,38 @@
180 in->reader() >> uri;182 in->reader() >> uri;
181183
182 auto reply = dbus::Message::make_method_return(in);184 auto reply = dbus::Message::make_method_return(in);
183 // Make sure the client has adequate apparmor permissions to open the URI185 uri_check->set(uri);
184 const auto result = request_authenticator->authenticate_open_uri_request(context, uri);186 const bool valid_uri = !uri_check->is_local_file() or
185 if (std::get<0>(result))187 (uri_check->is_local_file() and uri_check->file_exists());
186 {188 if (!valid_uri)
187 reply->writer() << (std::get<0>(result) ? impl->open_uri(uri) : false);189 {
188 }190 const std::string err_str = {"Warning: Failed to open uri " + uri +
189 else191 " because it can't be found."};
190 {
191 const std::string err_str = {"Warning: Failed to authenticate necessary "
192 "apparmor permissions to open uri: " + std::get<1>(result)};
193 std::cerr << err_str << std::endl;192 std::cerr << err_str << std::endl;
194 reply = dbus::Message::make_error(193 reply = dbus::Message::make_error(
195 in,194 in,
196 mpris::Player::Error::InsufficientAppArmorPermissions::name,195 mpris::Player::Error::UriNotFound::name,
197 err_str);196 err_str);
198 }197 }
198 else
199 {
200 // Make sure the client has adequate apparmor permissions to open the URI
201 const auto result = request_authenticator->authenticate_open_uri_request(context, uri);
202 if (std::get<0>(result))
203 {
204 reply->writer() << (std::get<0>(result) ? impl->open_uri(uri) : false);
205 }
206 else
207 {
208 const std::string err_str = {"Warning: Failed to authenticate necessary "
209 "apparmor permissions to open uri: " + std::get<1>(result)};
210 std::cerr << err_str << std::endl;
211 reply = dbus::Message::make_error(
212 in,
213 mpris::Player::Error::InsufficientAppArmorPermissions::name,
214 err_str);
215 }
216 }
199217
200 bus->send(reply);218 bus->send(reply);
201 });219 });
@@ -211,22 +229,38 @@
211 in->reader() >> uri >> headers;229 in->reader() >> uri >> headers;
212230
213 auto reply = dbus::Message::make_method_return(in);231 auto reply = dbus::Message::make_method_return(in);
214 // Make sure the client has adequate apparmor permissions to open the URI232 uri_check->set(uri);
215 const auto result = request_authenticator->authenticate_open_uri_request(context, uri);233 const bool valid_uri = !uri_check->is_local_file() or
216 if (std::get<0>(result))234 (uri_check->is_local_file() and uri_check->file_exists());
217 {235 if (!valid_uri)
218 reply->writer() << (std::get<0>(result) ? impl->open_uri(uri, headers) : false);236 {
219 }237 const std::string err_str = {"Warning: Failed to open uri " + uri +
220 else238 " because it can't be found."};
221 {
222 const std::string err_str = {"Warning: Failed to authenticate necessary "
223 "apparmor permissions to open uri: " + std::get<1>(result)};
224 std::cerr << err_str << std::endl;239 std::cerr << err_str << std::endl;
225 reply = dbus::Message::make_error(240 reply = dbus::Message::make_error(
226 in,241 in,
227 mpris::Player::Error::InsufficientAppArmorPermissions::name,242 mpris::Player::Error::UriNotFound::name,
228 err_str);243 err_str);
229 }244 }
245 else
246 {
247 // Make sure the client has adequate apparmor permissions to open the URI
248 const auto result = request_authenticator->authenticate_open_uri_request(context, uri);
249 if (std::get<0>(result))
250 {
251 reply->writer() << (std::get<0>(result) ? impl->open_uri(uri, headers) : false);
252 }
253 else
254 {
255 const std::string err_str = {"Warning: Failed to authenticate necessary "
256 "apparmor permissions to open uri: " + std::get<1>(result)};
257 std::cerr << err_str << std::endl;
258 reply = dbus::Message::make_error(
259 in,
260 mpris::Player::Error::InsufficientAppArmorPermissions::name,
261 err_str);
262 }
263 }
230264
231 bus->send(reply);265 bus->send(reply);
232 });266 });
@@ -258,6 +292,7 @@
258 dbus::Object::Ptr object;292 dbus::Object::Ptr object;
259 media::apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver;293 media::apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver;
260 media::apparmor::ubuntu::RequestAuthenticator::Ptr request_authenticator;294 media::apparmor::ubuntu::RequestAuthenticator::Ptr request_authenticator;
295 media::UriCheck::Ptr uri_check;
261296
262 mpris::Player::Skeleton skeleton;297 mpris::Player::Skeleton skeleton;
263298
264299
=== modified file 'src/core/media/player_stub.cpp'
--- src/core/media/player_stub.cpp 2016-01-14 19:47:40 +0000
+++ src/core/media/player_stub.cpp 2016-02-22 18:59:27 +0000
@@ -266,6 +266,8 @@
266 {266 {
267 if (op.error().name() == mpris::Player::Error::InsufficientAppArmorPermissions::name)267 if (op.error().name() == mpris::Player::Error::InsufficientAppArmorPermissions::name)
268 throw media::Player::Errors::InsufficientAppArmorPermissions{op.error().print()};268 throw media::Player::Errors::InsufficientAppArmorPermissions{op.error().print()};
269 else if (op.error().name() == mpris::Player::Error::UriNotFound::name)
270 throw media::Player::Errors::UriNotFound{op.error().print()};
269 else271 else
270 throw std::runtime_error{op.error().print()};272 throw std::runtime_error{op.error().print()};
271 }273 }
272274
=== modified file 'src/core/media/track_list_implementation.cpp'
--- src/core/media/track_list_implementation.cpp 2015-11-18 18:33:31 +0000
+++ src/core/media/track_list_implementation.cpp 2016-02-22 18:59:27 +0000
@@ -140,7 +140,6 @@
140 {140 {
141 auto it = std::find(container.begin(), container.end(), position);141 auto it = std::find(container.begin(), container.end(), position);
142 container.insert(it, id);142 container.insert(it, id);
143 std::cout << "container.size(): " << container.size() << std::endl;
144143
145 return true;144 return true;
146 });145 });
147146
=== modified file 'src/core/media/track_list_skeleton.cpp'
--- src/core/media/track_list_skeleton.cpp 2015-11-18 18:33:31 +0000
+++ src/core/media/track_list_skeleton.cpp 2016-02-22 18:59:27 +0000
@@ -27,7 +27,9 @@
27#include "track_list_traits.h"27#include "track_list_traits.h"
28#include "the_session_bus.h"28#include "the_session_bus.h"
2929
30#include "mpris/player.h"
30#include "mpris/track_list.h"31#include "mpris/track_list.h"
32#include "util/uri_check.h"
3133
32#include <core/dbus/object.h>34#include <core/dbus/object.h>
33#include <core/dbus/property.h>35#include <core/dbus/property.h>
@@ -56,6 +58,7 @@
56 object(object),58 object(object),
57 request_context_resolver(request_context_resolver),59 request_context_resolver(request_context_resolver),
58 request_authenticator(request_authenticator),60 request_authenticator(request_authenticator),
61 uri_check(std::make_shared<UriCheck>()),
59 skeleton(mpris::TrackList::Skeleton::Configuration{object, mpris::TrackList::Skeleton::Configuration::Defaults{}}),62 skeleton(mpris::TrackList::Skeleton::Configuration{object, mpris::TrackList::Skeleton::Configuration::Defaults{}}),
60 current_track(skeleton.properties.tracks->get().begin()),63 current_track(skeleton.properties.tracks->get().begin()),
61 empty_iterator(skeleton.properties.tracks->get().begin()),64 empty_iterator(skeleton.properties.tracks->get().begin()),
@@ -112,23 +115,39 @@
112115
113 // Make sure the client has adequate apparmor permissions to open the URI116 // Make sure the client has adequate apparmor permissions to open the URI
114 const auto result = request_authenticator->authenticate_open_uri_request(context, uri);117 const auto result = request_authenticator->authenticate_open_uri_request(context, uri);
115
116 auto reply = dbus::Message::make_method_return(msg);118 auto reply = dbus::Message::make_method_return(msg);
117 // Only add the track to the TrackList if it passes the apparmor permissions check119
118 if (std::get<0>(result))120 uri_check->set(uri);
119 {121 const bool valid_uri = !uri_check->is_local_file() or
120 impl->add_track_with_uri_at(uri, after, make_current);122 (uri_check->is_local_file() and uri_check->file_exists());
121 }123 if (!valid_uri)
122 else
123 {124 {
124 const std::string err_str = {"Warning: Not adding track " + uri +125 const std::string err_str = {"Warning: Not adding track " + uri +
125 " to TrackList because of inadequate client apparmor permissions."};126 " to TrackList because it can't be found."};
126 std::cerr << err_str << std::endl;127 std::cerr << err_str << std::endl;
127 reply = dbus::Message::make_error(128 reply = dbus::Message::make_error(
128 msg,129 msg,
129 mpris::TrackList::Error::InsufficientPermissionsToAddTrack::name,130 mpris::Player::Error::UriNotFound::name,
130 err_str);131 err_str);
131 }132 }
133 else
134 {
135 // Only add the track to the TrackList if it passes the apparmor permissions check
136 if (std::get<0>(result))
137 {
138 impl->add_track_with_uri_at(uri, after, make_current);
139 }
140 else
141 {
142 const std::string err_str = {"Warning: Not adding track " + uri +
143 " to TrackList because of inadequate client apparmor permissions."};
144 std::cerr << err_str << std::endl;
145 reply = dbus::Message::make_error(
146 msg,
147 mpris::TrackList::Error::InsufficientPermissionsToAddTrack::name,
148 err_str);
149 }
150 }
132151
133 bus->send(reply);152 bus->send(reply);
134 });153 });
@@ -144,10 +163,26 @@
144 media::Track::Id after;163 media::Track::Id after;
145 msg->reader() >> uris >> after;164 msg->reader() >> uris >> after;
146165
166 bool valid_uri = false;
147 media::apparmor::ubuntu::RequestAuthenticator::Result result;167 media::apparmor::ubuntu::RequestAuthenticator::Result result;
148 std::string err_str;168 std::string uri_err_str, err_str;
169 core::dbus::Message::Ptr reply;
149 for (const auto uri : uris)170 for (const auto uri : uris)
150 {171 {
172 uri_check->set(uri);
173 valid_uri = !uri_check->is_local_file() or
174 (uri_check->is_local_file() and uri_check->file_exists());
175 if (!valid_uri)
176 {
177 uri_err_str = {"Warning: Not adding track " + uri +
178 " to TrackList because it can't be found."};
179 std::cerr << uri_err_str << std::endl;
180 reply = dbus::Message::make_error(
181 msg,
182 mpris::Player::Error::UriNotFound::name,
183 err_str);
184 }
185
151 // Make sure the client has adequate apparmor permissions to open the URI186 // Make sure the client has adequate apparmor permissions to open the URI
152 result = request_authenticator->authenticate_open_uri_request(context, uri);187 result = request_authenticator->authenticate_open_uri_request(context, uri);
153 if (not std::get<0>(result))188 if (not std::get<0>(result))
@@ -158,7 +193,6 @@
158 }193 }
159 }194 }
160195
161 core::dbus::Message::Ptr reply;
162 // Only add the track to the TrackList if it passes the apparmor permissions check196 // Only add the track to the TrackList if it passes the apparmor permissions check
163 if (std::get<0>(result))197 if (std::get<0>(result))
164 {198 {
@@ -312,6 +346,7 @@
312 dbus::Object::Ptr object;346 dbus::Object::Ptr object;
313 media::apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver;347 media::apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver;
314 media::apparmor::ubuntu::RequestAuthenticator::Ptr request_authenticator;348 media::apparmor::ubuntu::RequestAuthenticator::Ptr request_authenticator;
349 media::UriCheck::Ptr uri_check;
315350
316 mpris::TrackList::Skeleton skeleton;351 mpris::TrackList::Skeleton skeleton;
317 TrackList::ConstIterator current_track;352 TrackList::ConstIterator current_track;
318353
=== modified file 'src/core/media/track_list_stub.cpp'
--- src/core/media/track_list_stub.cpp 2015-11-18 15:55:23 +0000
+++ src/core/media/track_list_stub.cpp 2016-02-22 18:59:27 +0000
@@ -25,6 +25,7 @@
25#include "track_list_traits.h"25#include "track_list_traits.h"
26#include "the_session_bus.h"26#include "the_session_bus.h"
2727
28#include "mpris/player.h"
28#include "mpris/track_list.h"29#include "mpris/track_list.h"
2930
30#include <core/dbus/property.h>31#include <core/dbus/property.h>
@@ -239,6 +240,8 @@
239 if (op.error().name() ==240 if (op.error().name() ==
240 mpris::TrackList::Error::InsufficientPermissionsToAddTrack::name)241 mpris::TrackList::Error::InsufficientPermissionsToAddTrack::name)
241 throw media::TrackList::Errors::InsufficientPermissionsToAddTrack{};242 throw media::TrackList::Errors::InsufficientPermissionsToAddTrack{};
243 else if (op.error().name() == mpris::Player::Error::UriNotFound::name)
244 throw media::Player::Errors::UriNotFound{op.error().print()};
242 else245 else
243 throw std::runtime_error{op.error().print()};246 throw std::runtime_error{op.error().print()};
244 }247 }
@@ -255,6 +258,8 @@
255 if (op.error().name() ==258 if (op.error().name() ==
256 mpris::TrackList::Error::InsufficientPermissionsToAddTrack::name)259 mpris::TrackList::Error::InsufficientPermissionsToAddTrack::name)
257 throw media::TrackList::Errors::InsufficientPermissionsToAddTrack{};260 throw media::TrackList::Errors::InsufficientPermissionsToAddTrack{};
261 else if (op.error().name() == mpris::Player::Error::UriNotFound::name)
262 throw media::Player::Errors::UriNotFound{op.error().print()};
258 else263 else
259 throw std::runtime_error{op.error().print()};264 throw std::runtime_error{op.error().print()};
260 }265 }
261266
=== added file 'src/core/media/util/uri_check.h'
--- src/core/media/util/uri_check.h 1970-01-01 00:00:00 +0000
+++ src/core/media/util/uri_check.h 2016-02-22 18:59:27 +0000
@@ -0,0 +1,160 @@
1/*
2 *
3 * This program is free software: you can redistribute it and/or modify it
4 * under the terms of the GNU Lesser General Public License version 3,
5 * as published by the Free Software Foundation.
6 *
7 * This program is distributed in the hope that it will be useful,
8 * but WITHOUT ANY WARRANTY; without even the implied warranty of
9 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
10 * GNU Lesser General Public License for more details.
11 *
12 * You should have received a copy of the GNU Lesser General Public License
13 * along with this program. If not, see <http://www.gnu.org/licenses/>.
14 *
15 * Authored by: Jim Hodapp <jim.hodapp@canonical.com>
16 *
17 */
18
19#ifndef URI_CHECK_H_
20#define URI_CHECK_H_
21
22#include <gio/gio.h>
23
24#include <memory>
25#include <iostream>
26
27namespace core
28{
29namespace ubuntu
30{
31namespace media
32{
33
34class UriCheck
35{
36public:
37 typedef std::shared_ptr<UriCheck> Ptr;
38
39 UriCheck()
40 : is_encoded_(false),
41 is_local_file_(false),
42 local_file_exists_(false)
43 {
44 }
45
46 UriCheck(const std::string& uri)
47 : uri_(uri),
48 is_encoded_(determine_if_encoded()),
49 is_local_file_(determine_if_local_file()),
50 local_file_exists_(determine_if_file_exists())
51 {
52 }
53
54 virtual ~UriCheck()
55 {
56 }
57
58 void set(const std::string& uri)
59 {
60 if (uri.empty())
61 return;
62
63 uri_ = uri;
64 is_encoded_ = determine_if_encoded();
65 is_local_file_ = determine_if_local_file();
66 local_file_exists_ = determine_if_file_exists();
67 }
68
69 void clear()
70 {
71 uri_.clear();
72 }
73
74 bool is_encoded() const
75 {
76 return is_encoded_;
77 }
78
79 bool is_local_file() const
80 {
81 return is_local_file_;
82 }
83
84 bool file_exists() const
85 {
86 return local_file_exists_;
87 }
88
89protected:
90 UriCheck(const UriCheck&) = delete;
91 UriCheck& operator=(const UriCheck&) = delete;
92
93private:
94 bool determine_if_encoded()
95 {
96 if (uri_.empty())
97 return false;
98
99 gchar *tmp = g_uri_unescape_string(uri_.c_str(), nullptr);
100 if (!tmp)
101 return false;
102
103 const std::string unescaped_uri{tmp};
104 g_free(tmp);
105 return unescaped_uri.length() < uri_.length();
106 }
107
108 bool determine_if_local_file()
109 {
110 if (uri_.empty())
111 return false;
112
113 gchar *tmp = g_uri_parse_scheme(uri_.c_str());
114 std::string uri_scheme;
115 if (tmp)
116 uri_scheme.assign(tmp);
117 g_free(tmp);
118 try {
119 return uri_.at(0) == '/' or
120 (uri_.at(0) == '.' and uri_.at(1) == '/') or
121 uri_scheme == "file";
122 }
123 catch (const std::out_of_range &e) {
124 std::cerr << "Invalid URI string provided: " << uri_ << std::endl;
125 return false;
126 }
127 }
128
129 bool determine_if_file_exists()
130 {
131 if (!is_local_file_)
132 return false;
133
134 GError *error = nullptr;
135 // Open the URI and get the mime type from it. This will currently only work for
136 // a local file
137 std::unique_ptr<GFile, void(*)(void *)> file(
138 g_file_new_for_uri(uri_.c_str()), g_object_unref);
139 std::unique_ptr<GFileInfo, void(*)(void *)> info(
140 g_file_query_info(
141 file.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE ","
142 G_FILE_ATTRIBUTE_ETAG_VALUE, G_FILE_QUERY_INFO_NONE,
143 /* cancellable */ NULL, &error),
144 g_object_unref);
145
146 return info.get() != nullptr;
147 }
148
149private:
150 std::string uri_;
151 bool is_encoded_;
152 bool is_local_file_;
153 bool local_file_exists_;
154};
155
156}
157}
158}
159
160#endif // URI_CHECK_H_

Subscribers

People subscribed via source and target branches

to all changes: