nsca-client delim chars MUST be specified in HEX now?
rdica opened this issue · 7 comments
I have scripts that pipe echo to send_nsca using comma as a delim char
-d ','
This has been working with nsca-client 2.9.2. When Fedora updated its rpm to use 2.10.0, my scripts broke with the dreaded 0 data packet(s) sent to host successfully. error. I filed a bug report at RH's bugzilla and during the investigation found that if I used the HEX value for comma: 0x2c, the data was sent successfully. I checked the Changelog and found no mention of the change requirement regarding delim chars. The only hint (other than having to read source code, something I'm not really capable of doing, not a programmer) was in the cli help that mentions "honoring HEX",
which to me, doesn't mean REQUIRED to use, but CAN be used.
CHANGELOG and command help should specify this as it's not at all clear it's now a requirement to use HEX value instead of the actual character.
This is not a documentation bug, this is a code bug. As I read it, the '-d' parameter was not supposed to change behavior, it was supposed to accept hex in addition to char.
It's really too late for me now, but this commit 644dc7d (and the 2 following fixes to it) looks really suspicious.
I can confirm hex delimiter handling is the problem. The following commit 8ca02b6 drops it and restores the previous behavior.
This is not meant to be committed as is, it would be better to fix the hex handling rather than purely drop it.
I will provide a proper Fedora/EPEL build with this patch applied as soon as @rdica has confirmed the scratch build I will provide in a few minutes fixes his use case.
I will try and provide a better patch as time permit or take upstream patch as soon as there is one.
I think this is a show-stopper for nsca 2.10.0 and claims for a 2.10.1 release asap, this bug is probably breaking a lot of people setup.
@sawolf, I have no other choice but to reverse to the previous behavior with the above patch. Thankfully, @rdica got hit in Fedora before the 2.10.0 build reached stable EPEL repo, which very likely has a lot more users. Anyone using -d in their scripts, which is likely very common practice, would have been left with a broken setup, just like @rdica was. We're unfortunately not using -d at $DAYJOB, so I didn't catch it myself even earlier when testing the 2.10.0 release.
Sorry about the long wait - bc909f7 resolved this issue in my testing. Let me know if you still see this issue after compiling from master. There haven't been any issues mentioned aside from this, so we'll put out a release once the QA team signs off on this change.
Tested OK here. Old -d behavior is fixed and the additional -d 0x.. works as well.
I find confusing though that -d 9 is treated the same as -d 0x9 but -d 2c is not the same as -d 0x2c or -d ,.
And if someone is using either [0-9] (unlikely) or + or - (slightly more chances) as delimiter, it won't work and none of this is documented as not possible.
That's a good point - the latest push to master will check if a single character is used, and will use the literal character if this is the case.
As for your second sentence (using -d 2c vs -d 0x2c), the original writer of the patch wanted to be able to use both decimal and hexadecimal numbers. So, if you're comparing something like -d 40 vs -d 0x40, those should absolutely be different (interpreted as ( vs @, respectively). If you have that behavior, then it's weird to allow cases like -d 2c to be treated as a hexadecimal number.