From d087d6dca056b5cb59368d9848a2aa3bb6d426be Mon Sep 17 00:00:00 2001 From: Tristan Matthews <tristan.matthews@savoirfairelinux.com> Date: Mon, 16 Jul 2012 14:00:14 -0400 Subject: [PATCH] * #13534: account: add proper validation for video codecs --- daemon/src/account.cpp | 71 ++++++++++++++++++++++++++++++++--- daemon/src/account.h | 4 +- daemon/src/sip/sipaccount.cpp | 5 ++- gnome/src/config/videoconf.c | 4 +- 4 files changed, 74 insertions(+), 10 deletions(-) diff --git a/daemon/src/account.cpp b/daemon/src/account.cpp index 80669ff2fa..6ff8cc0c64 100644 --- a/daemon/src/account.cpp +++ b/daemon/src/account.cpp @@ -38,6 +38,7 @@ #include "video/libav_utils.h" #endif +#include "logger.h" #include "manager.h" #include "dbus/configurationmanager.h" @@ -116,17 +117,77 @@ void Account::loadDefaultCodecs() setActiveAudioCodecs(result); #ifdef SFL_VIDEO - setVideoCodecs(libav_utils::getDefaultCodecs()); + // we don't need to validate via setVideoCodecs, since these are defaults + videoCodecList_ = libav_utils::getDefaultCodecs(); #endif } +#ifdef SFL_VIDEO +namespace { + bool isPositiveInteger(const string &s) + { + string::const_iterator it = s.begin(); + while (it != s.end() and std::isdigit(*it)) + ++it; + return not s.empty() and it == s.end(); + } + + bool isBoolean(const string &s) + { + return s == "true" or s == "false"; + } + + template <typename Predicate> + bool isFieldValid(const map<string, string> &codec, const char *field, Predicate p) + { + map<string, string>::const_iterator key(codec.find(field)); + return key != codec.end() and p(key->second); + } + + bool isCodecValid(const map<string, string> &codec, const vector<map<string, string> > &defaults) + { + map<string, string>::const_iterator name(codec.find(Account::VIDEO_CODEC_NAME)); + if (name == codec.end()) { + ERROR("Field \"name\" missing in codec specification"); + return false; + } + + // check that it's in the list of valid codecs and that it has all the required fields + for (vector<map<string, string> >::const_iterator i = defaults.begin(); i != defaults.end(); ++i) { + map<string, string>::const_iterator defaultName = i->find(Account::VIDEO_CODEC_NAME); + if (defaultName->second == name->second) { + return isFieldValid(codec, Account::VIDEO_CODEC_BITRATE, isPositiveInteger) + and isFieldValid(codec, Account::VIDEO_CODEC_ENABLED, isBoolean); + } + } + ERROR("Codec %s not supported", name->second.c_str()); + return false; + } + + bool isCodecListValid(const vector<map<string, string> > &list) + { + const vector<map<string, string> > defaults(libav_utils::getDefaultCodecs()); + if (list.size() != defaults.size()) { + ERROR("New codec list has a different length than the list of supported codecs"); + return false; + } + + // make sure that all codecs are present + for (vector<map<string, string> >::const_iterator i = list.begin(); + i != list.end(); ++i) { + if (not isCodecValid(*i, defaults)) + return false; + } + return true; + } +} +#endif + void Account::setVideoCodecs(const vector<map<string, string> > &list) { #ifdef SFL_VIDEO - // first clear the previously stored codecs - videoCodecList_.clear(); - // FIXME: do real validation here - videoCodecList_ = list; + if (isCodecListValid(list)) + videoCodecList_ = list; #else (void) list; #endif diff --git a/daemon/src/account.h b/daemon/src/account.h index 9c5c172e98..ee42256d8d 100644 --- a/daemon/src/account.h +++ b/daemon/src/account.h @@ -184,6 +184,8 @@ class Account : public Serializable { } static const char * const VIDEO_CODEC_ENABLED; + static const char * const VIDEO_CODEC_NAME; + static const char * const VIDEO_CODEC_BITRATE; private: NON_COPYABLE(Account); @@ -197,8 +199,6 @@ class Account : public Serializable { // General configuration keys for accounts static const char * const AUDIO_CODECS_KEY; static const char * const VIDEO_CODECS_KEY; - static const char * const VIDEO_CODEC_NAME; - static const char * const VIDEO_CODEC_BITRATE; static const char * const RINGTONE_PATH_KEY; static const char * const RINGTONE_ENABLED_KEY; static const char * const DISPLAY_NAME_KEY; diff --git a/daemon/src/sip/sipaccount.cpp b/daemon/src/sip/sipaccount.cpp index 3b22bc4251..03d02cbd5b 100644 --- a/daemon/src/sip/sipaccount.cpp +++ b/daemon/src/sip/sipaccount.cpp @@ -317,7 +317,7 @@ void SIPAccount::unserialize(const Conf::MappingNode &mapNode) if (seq->empty()) { // Video codecs are an empty list WARN("Loading default video codecs"); - setVideoCodecs(libav_utils::getDefaultCodecs()); + videoCodecList_ = libav_utils::getDefaultCodecs(); } else { vector<map<string, string> > videoCodecDetails; for (Sequence::iterator it = seq->begin(); it != seq->end(); ++it) { @@ -328,13 +328,14 @@ void SIPAccount::unserialize(const Conf::MappingNode &mapNode) codec->getValue(VIDEO_CODEC_ENABLED, &codecMap[VIDEO_CODEC_ENABLED]); videoCodecDetails.push_back(codecMap); } + // these must be validated setVideoCodecs(videoCodecDetails); } } else { // either this is an older config file which had videoCodecs as a scalar node, // or it had no video codecs at all WARN("Loading default video codecs"); - setVideoCodecs(libav_utils::getDefaultCodecs()); + videoCodecList_ = libav_utils::getDefaultCodecs(); } #endif diff --git a/gnome/src/config/videoconf.c b/gnome/src/config/videoconf.c index be262254c6..3639403712 100644 --- a/gnome/src/config/videoconf.c +++ b/gnome/src/config/videoconf.c @@ -366,10 +366,12 @@ bitrate_edited_cb(GtkCellRenderer *renderer UNUSED, gchar *path, gchar *new_text // Get active value and name at iteration const gint base = 10; gchar *endptr; - const unsigned long long val = strtoull(new_text, &endptr, base); + const long long val = strtoll(new_text, &endptr, base); /* Ignore if it's not a number */ if (*endptr != '\0') { WARN("Ignoring characters %s\n", val, endptr); + } else if (val < 0) { + WARN("Ignoring negative bitrate value"); } else { // Get path of edited codec GtkTreePath *tree_path = gtk_tree_path_new_from_string(path); -- GitLab