/hg/icedtea-web: Fixes for coverity issues discovered in RH1121549
Jiri Vanek
jvanek at redhat.com
Wed Jul 30 16:25:40 UTC 2014
and NEWS?!?!?
On 07/30/2014 06:11 PM, aazores at icedtea.classpath.org wrote:
> changeset 5672bb76595d in /hg/icedtea-web
> details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=5672bb76595d
> author: Andrew Azores <aazores at redhat.com>
> date: Wed Jul 30 12:11:20 2014 -0400
>
> Fixes for coverity issues discovered in RH1121549
>
> 2014-07-30 Andrew Azores <aazores at redhat.com>
>
> Fixes for coverity issues discovered in RH1121549
> * plugin/icedteanp/IcedTeaNPPlugin.cc (ITNP_New): print error message and
> return error if JVM fails to start.
> (NP_Initialize): fix missing argument to PLUGIN_ERROR when unable to
> create data directory
> * plugin/icedteanp/IcedTeaParseProperties.cc (get_log_dir): refactored to
> reduce duplicate code, use IcedTeaPluginUtils.create_dir, and enhanced
> warning messages
> * plugin/icedteanp/IcedTeaPluginUtils.cc (create_dir, is_directory): new
> functions
> * plugin/icedteanp/IcedTeaPluginUtils.h: same
> * plugin/icedteanp/IcedTeaScriptablePluginObject.cc (setProperty): do not
> erroneously redeclare java_result
> * tests/cpp-unit-tests/IcedTeaPluginUtilsTest.cc: (is_directory,
> create_dir) new tests. (file_exists) added assertion that directories
> satisfy file_exists
>
>
> diffstat:
>
> ChangeLog | 19 ++++++
> plugin/icedteanp/IcedTeaNPPlugin.cc | 6 +-
> plugin/icedteanp/IcedTeaParseProperties.cc | 39 +++++--------
> plugin/icedteanp/IcedTeaPluginUtils.cc | 38 +++++++++++++
> plugin/icedteanp/IcedTeaPluginUtils.h | 3 +
> plugin/icedteanp/IcedTeaScriptablePluginObject.cc | 2 +-
> tests/cpp-unit-tests/IcedTeaPluginUtilsTest.cc | 66 +++++++++++++++++++++++
> 7 files changed, 147 insertions(+), 26 deletions(-)
>
> diffs (276 lines):
>
> diff -r 6a985d697bf1 -r 5672bb76595d ChangeLog
> --- a/ChangeLog Mon Jul 28 12:36:26 2014 -0400
> +++ b/ChangeLog Wed Jul 30 12:11:20 2014 -0400
> @@ -1,3 +1,22 @@
> +2014-07-30 Andrew Azores <aazores at redhat.com>
> +
> + Fixes for coverity issues discovered in RH1121549
> + * plugin/icedteanp/IcedTeaNPPlugin.cc (ITNP_New): print error message and
> + return error if JVM fails to start.
> + (NP_Initialize): fix missing argument to PLUGIN_ERROR when unable to
> + create data directory
> + * plugin/icedteanp/IcedTeaParseProperties.cc (get_log_dir): refactored to
> + reduce duplicate code, use IcedTeaPluginUtils.create_dir, and enhanced
> + warning messages
> + * plugin/icedteanp/IcedTeaPluginUtils.cc (create_dir, is_directory): new
> + functions
> + * plugin/icedteanp/IcedTeaPluginUtils.h: same
> + * plugin/icedteanp/IcedTeaScriptablePluginObject.cc (setProperty): do not
> + erroneously redeclare java_result
> + * tests/cpp-unit-tests/IcedTeaPluginUtilsTest.cc: (is_directory,
> + create_dir) new tests. (file_exists) added assertion that directories
> + satisfy file_exists
> +
> 2014-07-28 Jie Kang <jkang at redhat.com>
> Fixed TeeOutputStream to accept multi-byte encodings.
> * netx/net/sourceforge/jnlp/util/logging/TeeOutputStream.java: Now uses
> diff -r 6a985d697bf1 -r 5672bb76595d plugin/icedteanp/IcedTeaNPPlugin.cc
> --- a/plugin/icedteanp/IcedTeaNPPlugin.cc Mon Jul 28 12:36:26 2014 -0400
> +++ b/plugin/icedteanp/IcedTeaNPPlugin.cc Wed Jul 30 12:11:20 2014 -0400
> @@ -375,6 +375,10 @@
>
> // start the jvm if needed
> NPError startup_error = start_jvm_if_needed();
> + if (startup_error != NPERR_NO_ERROR) {
> + PLUGIN_ERROR ("Failed to start JVM\n");
> + return startup_error;
> + }
>
> // Initialize data->instance_id.
> //
> @@ -2027,7 +2031,7 @@
> NPError np_error = initialize_data_directory();
> if (np_error != NPERR_NO_ERROR)
> {
> - PLUGIN_ERROR("Unable create data directory %s\n");
> + PLUGIN_ERROR("Unable to create data directory %s\n", data_directory.c_str());
> return np_error;
> }
>
> diff -r 6a985d697bf1 -r 5672bb76595d plugin/icedteanp/IcedTeaParseProperties.cc
> --- a/plugin/icedteanp/IcedTeaParseProperties.cc Mon Jul 28 12:36:26 2014 -0400
> +++ b/plugin/icedteanp/IcedTeaParseProperties.cc Wed Jul 30 12:11:20 2014 -0400
> @@ -123,39 +123,30 @@
> return string(mypasswd->pw_dir)+"/.config/icedtea-web/"+default_file_ITW_deploy_props_name;
> }
>
> -string get_log_dir(){
> +string get_log_dir(){
> string value;
> if (!read_deploy_property_value("deployment.user.logdir", value)) {
> - int myuid = getuid();
> - struct passwd *mypasswd = getpwuid(myuid);
> - // try pre 1.5 file location
> + string config_dir;
> if (getenv ("XDG_CONFIG_HOME") != NULL){
> - string r1= string(getenv ("XDG_CONFIG_HOME"))+"/icedtea-web";
> - string r2 = r1+"/"+default_itw_log_dir_name;
> - if (!IcedTeaPluginUtilities::file_exists(r1)){
> - g_mkdir(r1.c_str(), 755);
> - }
> - if (!IcedTeaPluginUtilities::file_exists(r2)){
> - g_mkdir(r2.c_str(), 755);
> - }
> - return r2;
> + config_dir = string(getenv("XDG_CONFIG_HOME"));
> + } else {
> + int myuid = getuid();
> + struct passwd *mypasswd = getpwuid(myuid);
> + config_dir = string(mypasswd->pw_dir) + "/.config";
> }
> - //if not then use default
> - string r1 = string(mypasswd->pw_dir)+"/.config/icedtea-web";
> - string r2 = r1+"/"+default_itw_log_dir_name;
> - if (!IcedTeaPluginUtilities::file_exists(r1)){
> - g_mkdir(r1.c_str(), 755);
> + string itw_dir = config_dir+"/icedtea-web";
> + string log_dir = itw_dir+"/"+default_itw_log_dir_name;
> + bool created_config = IcedTeaPluginUtilities::create_dir(itw_dir);
> + bool created_log = IcedTeaPluginUtilities::create_dir(log_dir);
> + if (!created_config || !created_log){
> + PLUGIN_ERROR("IcedTea-Web log directory creation failed. IcedTea-Web may fail to work!");
> }
> - if (!IcedTeaPluginUtilities::file_exists(r2)){
> - g_mkdir(r2.c_str(), 755);
> - }
> - return r2;
> + return log_dir;
> }
> return value;
> }
>
> -
> -string main_properties_file(){
> +string main_properties_file(){
> return "/etc/.java/deployment/"+default_file_ITW_deploy_props_name;
> }
>
> diff -r 6a985d697bf1 -r 5672bb76595d plugin/icedteanp/IcedTeaPluginUtils.cc
> --- a/plugin/icedteanp/IcedTeaPluginUtils.cc Mon Jul 28 12:36:26 2014 -0400
> +++ b/plugin/icedteanp/IcedTeaPluginUtils.cc Wed Jul 30 12:11:20 2014 -0400
> @@ -59,6 +59,33 @@
> /* Plugin async call queue */
> static std::vector< PluginThreadCall* >* pendingPluginThreadRequests = new std::vector< PluginThreadCall* >();
>
> +bool
> +IcedTeaPluginUtilities::create_dir(std::string dir)
> +{
> + if (file_exists(dir))
> + {
> + if (!is_directory(dir))
> + {
> + PLUGIN_ERROR("WARNING: Needed to create directory %s but there is already a file of the same name at this location.\n", dir.c_str());
> + return false;
> + }
> + PLUGIN_DEBUG("Directory %s already exists\n", dir.c_str());
> + } else
> + {
> + PLUGIN_DEBUG("Directory %s does not yet exist\n", dir.c_str());
> + const int PERMISSIONS_MASK = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH; // 0755
> + bool created_directory = (g_mkdir(dir.c_str(), PERMISSIONS_MASK) == 0);
> + int err = errno;
> + if (!created_directory)
> + {
> + PLUGIN_ERROR("WARNING: Failed to create new directory %s. Reason: %s\n", dir.c_str(), strerror(err));
> + return false;
> + }
> + PLUGIN_DEBUG("Directory %s created\n", dir.c_str());
> + }
> + return true;
> +}
> +
> void *flush_pre_init_messages(void* data) {
> while (true){
> struct timespec ts;
> @@ -1136,6 +1163,17 @@
> return infile.good();
> }
>
> +bool IcedTeaPluginUtilities::is_directory(std::string filename)
> +{
> + if (!file_exists)
> + {
> + return false;
> + }
> + struct stat buf;
> + stat(filename.c_str(), &buf);
> + return S_ISDIR(buf.st_mode);
> +}
> +
> void IcedTeaPluginUtilities::initFileLog(){
> if (plugin_file_log != NULL ) {
> //reusing
> diff -r 6a985d697bf1 -r 5672bb76595d plugin/icedteanp/IcedTeaPluginUtils.h
> --- a/plugin/icedteanp/IcedTeaPluginUtils.h Mon Jul 28 12:36:26 2014 -0400
> +++ b/plugin/icedteanp/IcedTeaPluginUtils.h Wed Jul 30 12:11:20 2014 -0400
> @@ -49,6 +49,7 @@
> #include <time.h>
> #include <syslog.h>
> #include <sys/time.h>
> +#include <sys/stat.h>
>
> #include <fcntl.h>
> #include <cstring>
> @@ -430,12 +431,14 @@
> /*cutting whitespaces from end and start of string*/
> static void trim(std::string& str);
> static bool file_exists(std::string filename);
> + static bool is_directory(std::string filename);
> //file-loggers helpers
> static std::string generateLogFileName();
> static void initFileLog();
> static void printDebugStatus();
> static std::string getTmpPath();
> static std::string getRuntimePath();
> + static bool create_dir(std::string);
> };
>
> /*
> diff -r 6a985d697bf1 -r 5672bb76595d plugin/icedteanp/IcedTeaScriptablePluginObject.cc
> --- a/plugin/icedteanp/IcedTeaScriptablePluginObject.cc Mon Jul 28 12:36:26 2014 -0400
> +++ b/plugin/icedteanp/IcedTeaScriptablePluginObject.cc Wed Jul 30 12:11:20 2014 -0400
> @@ -704,7 +704,7 @@
> browser_functions.intfromidentifier(name_id) >= 0) // else if array and requesting index
> {
>
> - JavaResultData* java_result = java_request.getArrayLength(instance_id);
> + java_result = java_request.getArrayLength(instance_id);
> if (java_result->error_occurred)
> {
> PLUGIN_ERROR("ERROR: Couldn't fetch array length\n");
> diff -r 6a985d697bf1 -r 5672bb76595d tests/cpp-unit-tests/IcedTeaPluginUtilsTest.cc
> --- a/tests/cpp-unit-tests/IcedTeaPluginUtilsTest.cc Mon Jul 28 12:36:26 2014 -0400
> +++ b/tests/cpp-unit-tests/IcedTeaPluginUtilsTest.cc Wed Jul 30 12:11:20 2014 -0400
> @@ -135,6 +135,72 @@
> remove(f1.c_str());
> bool b = IcedTeaPluginUtilities::file_exists(f1);
> CHECK_EQUAL(b, false);
> +
> + std::string dir = tmpnam(NULL);
> + const int PERMISSIONS_MASK = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH; // 0755
> + bool created_dir = g_mkdir(dir.c_str(), PERMISSIONS_MASK);
> + CHECK_EQUAL(created_dir, false);
> + CHECK_EQUAL(IcedTeaPluginUtilities::file_exists(dir), true);
> +}
> +
> +
> +TEST(is_directory) {
> + std::string n = tmpnam(NULL);
> + bool no_file = IcedTeaPluginUtilities::is_directory(n);
> + CHECK_EQUAL(no_file, false);
> +
> + std::string f = temporary_file("dummy content");
> + bool is_directory = IcedTeaPluginUtilities::is_directory(f);
> + CHECK_EQUAL(is_directory, false);
> +
> + std::string d = tmpnam(NULL);
> + const int PERMISSIONS_MASK = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH; // 0755
> + bool created_test_dir = g_mkdir(d.c_str(), PERMISSIONS_MASK);
> + CHECK_EQUAL(created_test_dir, false);
> + bool is_directory2 = IcedTeaPluginUtilities::is_directory(d);
> + CHECK_EQUAL(is_directory2, true);
> +}
> +
> +
> +TEST(create_dir) {
> + FILE* old1 = stdout;
> + FILE* old2 = stderr;
> + char* buf1 = " ";
> + char* buf2 = " ";
> + stdout = fmemopen (buf1, strlen (buf1), "rw");
> + stderr = fmemopen (buf2, strlen (buf2), "rw");
> +
> + std::string f1 = tmpnam(NULL);
> + bool res1 = IcedTeaPluginUtilities::create_dir(f1);
> + CHECK_EQUAL(res1, true);
> + CHECK_EQUAL(IcedTeaPluginUtilities::is_directory(f1), true);
> +
> + std::string f2 = temporary_file("dummy content");
> + bool res2 = IcedTeaPluginUtilities::create_dir(f2);
> + CHECK_EQUAL(res2, false);
> + CHECK_EQUAL(IcedTeaPluginUtilities::is_directory(f2), false);
> +
> + std::string f3 = tmpnam(NULL);
> + const int PERMISSIONS_MASK = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH; // 0755
> + bool created_test_dir = g_mkdir(f3.c_str(), PERMISSIONS_MASK);
> + CHECK_EQUAL(created_test_dir, false);
> + bool res3 = IcedTeaPluginUtilities::create_dir(f3);
> + CHECK_EQUAL(res3, true);
> + CHECK_EQUAL(IcedTeaPluginUtilities::is_directory(f3), true);
> +
> + std::string f4 = tmpnam(NULL);
> + const int READONLY_PERMISSIONS_MASK = S_IRUSR | S_IRGRP | S_IROTH; // 0444
> + bool created_test_dir2 = g_mkdir(f4.c_str(), READONLY_PERMISSIONS_MASK);
> + CHECK_EQUAL(created_test_dir2, false);
> + std::string subdir = f4 + "/test";
> + bool res4 = IcedTeaPluginUtilities::create_dir(subdir);
> + CHECK_EQUAL(res4, false);
> + CHECK_EQUAL(IcedTeaPluginUtilities::is_directory(subdir), false);
> +
> + fclose(stdout);
> + fclose(stderr);
> + stdout = old1;
> + stderr = old2;
> }
>
>
>
More information about the distro-pkg-dev
mailing list