On rawhide today, "pmconfig -L | grep nss" returns the string
nss_version=3.16 Basic ECC
And this is not acceptable to all the uses of
eval 'pmconfig -L ...'
throughout the testsuite ("Basic: command not found").
Looking at the insides of pmconfig.c, there is no awareness of strings
possibly containing spaces or special characters (like " or \n). The
-l and -s options aren't documented, and aren't implemented enough to
quote general strings anyhow.
It looks like we have a total of two users of pmconfig in the source
tree. Both appear to want bourne-shell-compatible quoting on the
output (-s for mingw, -L for pcpqa), but don't get it. We should
probably unconditionally (and more carefully) quote the output strings.
Oh wow, unexpected! I've not come across an nss_version thats not just the dotted-number variety before. But, on inspection of the headers, yes they have always had this possibility it seems...
/usr/include/nss3/nss.h:#define NSS_VERSION "126.96.36.199" _NSS_ECC_STRING _NSS_CUSTOMIZED
... we've just not come across either of those last two macros expand in any shipped product we've seen before until now.
| It looks like we have a total of two users of pmconfig in the source
| tree. Both appear to want bourne-shell-compatible quoting on the
| output (-s for mingw, -L for pcpqa), but don't get it. We should
Its primarily used in QA, where there are some 35 tests using it & one of the common.* files (used by 3/4 more tests IIRC).
This is more complicated to fix than it at first seems, as pmconfig -s mode will add the quoting in some situations (though, not sure its always doing what I thought it would do) in which case, we'll get double quoting and breakage will probably result. Will dig a little deeper, bit of a rats nest.
The new pmconfig.c code in commit 7c3f89afe22 improves this greatly.
It should probably go a little further in quoting or precluding
questionable strings, such as those that contain " or \n characters.
(For these printf("%s=\"%s\"", key, value); is not enough.)
*nod* - will do - thanks for reviewing. The back-tick character also needs quoting I suppose, possibly the dollar sign, round/curly braces. Any others?
We will have successfully shot ourselves in the foot if any of those ever make it from pcp.conf or hard-coded-in-libpcp out to this point, but certainly there's never harm in extra defence against the dark arts.
The new logic from commit 2ad34572 is even better, yup, though
I believe it misses \ itself. Perhaps instead of hunting for a
complete blacklist of problematic characters, consider the
whitelist approach, as embodied in pmmgr.cxx sh_quote().
Excellent point re '\' - re-reviewing the pmconfig change, it seems it is quoting far too much, in fact. There appears to be different expansion rules applied in the shell depending on whether it is expanding a ""-enclosed string to everything else. As such, the pmmgr.cxx code also looks like it will incorrectly quote strings as well. For example:
nathans@smash:~$ echo "(sdf"
nathans@smash:~$ echo "\(sdf"
This would be an inappropriate transformation for a string value, right? (cos that back-quote will now be passed into the called program, instead of shell-expanded, in the case of pmmgr.cxx sh_quote). Gah, its never easy! :)
[ this appears to be an issue in pmmgr handling of string constants vs other shell-expanded stuff? hopefully, I'm overlooking some other magic its doing ]
From further experimentation, the list of characters needing quoting in the pmconfig string constant case is actually very small (smaller than I originally thought, although \ and ! were missed - none of the braces need quoting, as long as dollar-sign is correctly escaped).
Fascinating stuff though, I'm learning, I'm learning - thanks for following up!
re #c5 - Frank points out on IRC that its all good in pmmgr, as it cleverly is quoting the string start/end too, thus there is no string-constant case like in pmconfig - so all should be well in pmmgr.
pcp-3.9.2-1.el6 has been submitted as an update for Fedora EPEL 6.
pcp-3.9.2-1.el5 has been submitted as an update for Fedora EPEL 5.
pcp-3.9.2-1.fc20 has been submitted as an update for Fedora 20.
pcp-3.9.2-1.fc19 has been submitted as an update for Fedora 19.
* should fix your issue,
* was pushed to the Fedora EPEL 5 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=epel-testing pcp-3.9.2-1.el5'
as soon as you are able to.
Please go to the following url:
then log in and leave karma (feedback).
pcp-3.9.2-1.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.
pcp-3.9.2-1.fc20 has been pushed to the Fedora 20 stable repository. If problems still persist, please make note of it in this bug report.
pcp-3.9.2-1.fc19 has been pushed to the Fedora 19 stable repository. If problems still persist, please make note of it in this bug report.
pcp-3.9.2-1.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report.