Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Counter decrement functionality causes error #357

Closed
admsmith6 opened this issue Oct 28, 2013 · 3 comments
Closed

Counter decrement functionality causes error #357

admsmith6 opened this issue Oct 28, 2013 · 3 comments

Comments

@admsmith6
Copy link

'+' and '-' characters are not allowed in counter packets, preventing a client from decrementing a counter. I tested this bug using my own c++ client, as well as the python client from the "examples" folder. In both cases I get a bad line error sending a packet of the form:

testVar:-1|c

The problem seems to be in the lib/helpers.js file and was a result of commit 53fd384 which disallowed the '+' and '-' characters for all but gauge packets. A simple fix could be to add fields[1] == 'c' as an additional condition to the if (fields[1] == 'g') statement since the format for both counters and gauges seem to be identical.

@mrtazz
Copy link
Member

mrtazz commented Nov 6, 2013

Yeah seems I made the checks too tight in the helpers. It's related to #350

SaveTheRbtz pushed a commit to SaveTheRbtz/statsd that referenced this issue Dec 31, 2013
This commit introduces following changes:
 * Be liberal in what you recieve - do not rely on regexps for packet
   parsing but instead accept anything that can be parsed by JavaScript
   as a number e.g: +1e-17, -0511, 0xDEADbeef, etc
 * Accept both positive and negative counters with explicit / implicit
   sign
 * Provides more strict error checking then regexp, for example
   following strings match previously used '([\-\+\d\.]+' regexp:
   .
   .123.
   .\-+\0\+-.1-

Also while here added more tests.

Closes: statsd#350 statsd#357

Signed-off-by: Alexey Ivanov <SaveTheRbtz@GMail.com>
Signed-off-by: Alexey Ivanov <aivanov@linkedin.com>
SaveTheRbtz pushed a commit to SaveTheRbtz/statsd that referenced this issue Dec 31, 2013
This commit introduces following changes:
 * Be liberal in what you recieve - do not rely on regexps for packet
   parsing but instead accept anything that can be parsed by JavaScript
   as a number e.g: +1e-17, -0511, 0xDEADbeef, etc
 * Accept both positive and negative counters with explicit / implicit
   sign
 * Provides more strict error checking then regexp, for example
   following strings match previously used '([\-\+\d\.]+' regexp:
   .
   .123.
   .\-+\0\+-.1-

Also while here added more tests.

Closes: statsd#350 statsd#357

Signed-off-by: Alexey Ivanov <SaveTheRbtz@GMail.com>
SaveTheRbtz pushed a commit to SaveTheRbtz/statsd that referenced this issue Dec 31, 2013
This commit introduces following changes:
 * Be liberal in what you recieve - do not rely on regexps for packet
   parsing but instead accept anything that can be parsed by JavaScript
   as a number e.g: +1e-17, -0511, 0xDEADbeef, etc
 * Accept both positive and negative counters with explicit / implicit
   sign
 * Provides more strict error checking then regexp, for example
   following strings match previously used '([\-\+\d\.]+' regexp:
   .
   .123.
   .\-+\0\+-.1-

Also while here added more tests.

Closes: statsd#350 statsd#357

Signed-off-by: Alexey Ivanov <SaveTheRbtz@GMail.com>
@fjorgemota
Copy link

+1 for this bug, i'm affected using the version 0.7.1 of StatsD.

Any prevision of when this issue will be fixed? I've seen #350 but seems like the PR is not being updated :///

PS: I've seem #382 (which is actually merged (?)) but seems like the changes that it proposes is reverted before the release of v0.7.1, any info about it?

@mrtazz
Copy link
Member

mrtazz commented Apr 14, 2014

this is fixed with #382 and the changes are in master

@mrtazz mrtazz closed this as completed Apr 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants