[Merge] lp:~phablet-team/media-hub/fix-1449790 into lp:media-hub
Alfonso Sanchez-Beato
alfonso.sanchez-beato at canonical.com
Mon Feb 22 17:12:11 UTC 2016
Review: Needs Fixing
Still some comments, see below.
Diff comments:
>
> === 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 16:17:19 +0000
> @@ -633,17 +637,97 @@
> /* cancellable */ NULL, &error),
> g_object_unref);
> if (!info)
> - {
> - std::string error_str(error->message);
> - g_error_free(error);
> -
> - std::cout << "Failed to query the URI for the presence of video content: "
> - << error_str << std::endl;
> return std::string();
> - }
>
> - std::string content_type(g_file_info_get_attribute_string(
> + return std::string(g_file_info_get_attribute_string(
> info.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE));
> +}
> +
> +std::string gstreamer::Playbin::encode_uri(const std::string& uri) const
> +{
> + if (uri.empty())
> + return std::string();
> +
> + std::string encoded_uri;
> + media::UriCheck::Ptr uri_check{std::make_shared<media::UriCheck>(uri)};
> + gchar *uri_scheme = g_uri_parse_scheme(uri.c_str());
> + // We have a URI and it is already percent encoded
> + if (strlen(uri_scheme) > 0 and uri_check->is_encoded())
g_uri_parse_scheme() will return NULL if this is not a real URI ( see http://sourcecodebrowser.com/glib2.0/2.20.1/gurifuncs_8c_source.html ), so we need to check that before calling strlen().
> + {
> +#ifdef VERBOSE_DEBUG
> + std::cout << "Is a URI and is already percent encoded" << std::endl;
> +#endif
> + encoded_uri = uri;
> + }
> + // We have a URI but it's not already percent encoded
> + else if (strlen(uri_scheme) > 0 and !uri_check->is_encoded())
> + {
> +#ifdef VERBOSE_DEBUG
> + std::cout << "Is a URI and is not already percent encoded" << std::endl;
> +#endif
> + gchar *encoded = g_uri_escape_string(uri.c_str(),
> + "!$&'()*+,;=:/?[]@", // reserved chars
> + TRUE); // Allow UTF-8 chars
> + encoded_uri.assign(encoded);
> + g_free(encoded);
> + }
> + else // We have a path and not a URI. Turn it into a full URI and encode it
> + {
> + GError *error = nullptr;
> +#ifdef VERBOSE_DEBUG
> + std::cout << "Is a path and is not already percent encoded" << std::endl;
> +#endif
> + gchar *str = g_filename_to_uri(uri.c_str(), nullptr, &error);
> + encoded_uri.assign(str);
> + g_free(str);
> + if (error != nullptr)
> + {
> + std::cerr << "Warning: failed to get actual track content type: " << error->message
> + << std::endl;
> + g_error_free(error);
> + g_free(str);
> + g_free(uri_scheme);
> + return std::string("audio/video/");
> + }
> + encoded_uri.assign(g_uri_escape_string(encoded_uri.c_str(),
Value returned by g_uri_escape_string() needs to be freed.
> + "!$&'()*+,;=:/?[]@", // reserved chars
> + TRUE)); // Allow UTF-8 chars
> + }
> +
> + g_free(uri_scheme);
> +
> + return encoded_uri;
> +}
> +
> +std::string gstreamer::Playbin::decode_uri(const std::string& uri) const
> +{
> + if (uri.empty())
> + return std::string();
> +
> + gchar *decoded_gchar = g_uri_unescape_string(uri.c_str(), nullptr);
> + if (!decoded_gchar)
> + return std::string();
> +
> + const std::string decoded{decoded_gchar};
> + g_free(decoded_gchar);
> + return decoded;
> +}
> +
> +std::string gstreamer::Playbin::get_file_content_type(const std::string& uri) const
> +{
> + if (uri.empty())
> + return std::string();
> +
> + const std::string encoded_uri{encode_uri(uri)};
> +
> + const std::string content_type {file_info_from_uri(encoded_uri)};
> + if (content_type.empty())
> + {
> + std::cerr << "Warning: failed to get actual track content type" << std::endl;
> + return std::string("audio/video/");
> + }
> +
> + std::cout << "Found content type: " << content_type << std::endl;
>
> return content_type;
> }
>
> === 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 16:17:19 +0000
> @@ -0,0 +1,152 @@
> +/*
> + *
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License version 3,
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Authored by: Jim Hodapp <jim.hodapp at canonical.com>
> + *
> + */
> +
> +#ifndef URI_CHECK_H_
> +#define URI_CHECK_H_
> +
> +#include <gio/gio.h>
> +
> +#include <memory>
> +#include <iostream>
> +
> +namespace core
> +{
> +namespace ubuntu
> +{
> +namespace media
> +{
> +
> +class UriCheck
> +{
> +public:
> + typedef std::shared_ptr<UriCheck> Ptr;
> +
> + UriCheck()
> + : is_encoded_(false),
> + is_local_file_(false),
> + local_file_exists_(false)
> + {
> + }
> +
> + UriCheck(const std::string& uri)
> + : uri_(uri),
> + is_encoded_(determine_if_encoded()),
> + is_local_file_(determine_if_local_file()),
> + local_file_exists_(determine_if_file_exists())
> + {
> + }
> +
> + virtual ~UriCheck()
> + {
> + }
> +
> + void set(const std::string& uri)
> + {
> + if (uri.empty())
> + return;
> +
> + uri_ = uri;
> + is_encoded_ = determine_if_encoded();
> + is_local_file_ = determine_if_local_file();
> + local_file_exists_ = determine_if_file_exists();
> + }
> +
> + void clear()
> + {
> + uri_.clear();
> + }
> +
> + bool is_encoded() const
> + {
> + return is_encoded_;
> + }
> +
> + bool is_local_file() const
> + {
> + return is_local_file_;
> + }
> +
> + bool file_exists() const
> + {
> + return local_file_exists_;
> + }
> +
> +protected:
> + UriCheck(const UriCheck&) = delete;
> + UriCheck& operator=(const UriCheck&) = delete;
> +
> +private:
> + bool determine_if_encoded()
> + {
> + if (uri_.empty())
> + return false;
> +
> + gchar *tmp = g_uri_unescape_string(uri_.c_str(), nullptr);
> + if (!tmp)
> + return false;
> +
> + const std::string unescaped_uri{tmp};
> + g_free(tmp);
> + return unescaped_uri.length() < uri_.length();
> + }
> +
> + bool determine_if_local_file()
> + {
> + if (uri_.empty())
> + return false;
> +
> + gchar *tmp = g_uri_parse_scheme(uri_.c_str());
g_uri_parse_scheme() might return NULL, and in fact it will if uri is not a fully formed uri (see http://sourcecodebrowser.com/glib2.0/2.20.1/gurifuncs_8c_source.html , will certainly return NULL if there is no colon in the string for instance).
> + const std::string uri_scheme{tmp};
> + g_free(tmp);
> + return uri_.at(0) == '/' or
> + (uri_.at(0) == '.' and uri_.at(1) == '/') or
> + uri_scheme == "file";
After taking a look at the source code for g_uri_parse_scheme() I think this can crash with some strings (see comment above). Is uri_ supposed to be a fully formed URI or not?
> + }
> +
> + bool determine_if_file_exists()
> + {
> + if (!is_local_file_)
> + return false;
> +
> + GError *error = nullptr;
> + // Open the URI and get the mime type from it. This will currently only work for
> + // a local file
> + std::unique_ptr<GFile, void(*)(void *)> file(
> + g_file_new_for_uri(uri_.c_str()), g_object_unref);
> + std::unique_ptr<GFileInfo, void(*)(void *)> info(
> + g_file_query_info(
> + file.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE ","
> + G_FILE_ATTRIBUTE_ETAG_VALUE, G_FILE_QUERY_INFO_NONE,
> + /* cancellable */ NULL, &error),
> + g_object_unref);
> +
> + return info.get() != nullptr;
> + }
> +
> +private:
> + std::string uri_;
> + bool is_encoded_;
> + bool is_local_file_;
> + bool local_file_exists_;
> +};
> +
> +}
> +}
> +}
> +
> +#endif // URI_CHECK_H_
--
https://code.launchpad.net/~phablet-team/media-hub/fix-1449790/+merge/285813
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.
More information about the Ubuntu-reviews
mailing list