Mailinglist Archive: yast-devel (78 mails)

< Previous Next >
[yast-devel] Re: [yast-commit] r66316 - in /trunk/snapper: VERSION agent-snapper/src/SnapperAgent.cc agent-snapper/src/SnapperAgent.h package/yast2-snapper.changes src/Snapper.ycp src/dialogs.ycp
Dne 7.10.2011 11:16, jsuchome@xxxxxxxxxxxxxxxxx napsal(a):

-string SnapperAgent::getValue (const YCPMap map, const string key)
+string SnapperAgent::getValue (const YCPMap map, const string key, string
deflt)
{
if (!map->value(YCPString(key)).isNull()
&& map->value(YCPString(key))->isString())
return map->value(YCPString(key))->asString()->value();
else
- return "";
+ return deflt;
}


The "key" parameter is not changed so you could pass a reference to it
("const string &key"), that will prevent from creating unnecessary copy
of the string on function call.

"deflt" is also not changed, passing "const string &deflt" would help

(I'm not sure, I'm not C++ expert, but then you might need to create the copy
explicitly via

return string(deflt);

at the end.)

@@ -252,13 +282,10 @@
/**
* Return the given file mode as octal number
*/
-global integer GetFileMode (string file) {
+integer GetFileMode (string file) {

map out = (map) SCR::Execute (.target.bash_output, "/bin/stat
--printf=%a " + file);

When building custom shell commands String::Quote() should be always used, it
prevents from errors when the input contains spaces and improves security
(command injection) if the input comes from user.

if (Popup::AnyQuestionRichText (_("Restoring files"),
// popup message, %1 is snapshot number, %2 list of files
sformat (_("These files will be copied from snapshot '%1' to
current system: <p>%2</p>Are you sure?"),
- previous_num, mergestring (files, "<br>")),
+ previous_num, mergestring (to_restore, "<br>")),

Similar problem, although very unlikely, if a file name contains an HTML tag it
would
be wrongly displayed. Use String::EscapeTags().

- SCR::Execute (.target.bash, sformat ("/bin/cp -a '%1' '%2'", orig, file));
+ SCR::Execute (.target.bash, sformat ("/bin/cp -a '%1' '%2'", orig,
full_path));

String::Quote() should be also used here, '%1' helps when there is a space,
but doesn't help when the string contains ' (a single quote).

And if the command supports (cp does) we should use -- option to stop the option
processing. If %1 begins with -- it would be processed as an option, in some
cases
this can completely change the meaning of the command.

The most fool proof way is to execute shell commands like this:

SCR::Execute (.target.bash, sformat ("/bin/cp -a -- '%1' '%2'",
String::Quote(orig),
String::Quote(full_path)));


(Note: These problems are not critical in YaST because YaST is started by root,
but it's more critical in webyast and therefore we should get used to think
about these possible problems.)


--

Ladislav Slez√°k
Appliance department / YaST Developer
Lihovarsk√° 1060/12
190 00 Prague 9 / Czech Republic
tel: +420 284 028 960
lslezak@xxxxxxxx
SUSE
--
To unsubscribe, e-mail: yast-devel+unsubscribe@xxxxxxxxxxxx
To contact the owner, e-mail: yast-devel+owner@xxxxxxxxxxxx

< Previous Next >