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

support pickle protocol versions 0, 1, 2, 3 & 4 + accept pickle arrays + send pickle tuples #341

Merged
merged 3 commits into from
Feb 6, 2019

Conversation

DanCech
Copy link
Contributor

@DanCech DanCech commented Jan 22, 2019

Fixes #340

We have this check in place to drop obviosuly-invalid data sent to the pickle port, rather than trying to feed it into og-rek.

With this update we should cover all possible valid payload prefixes for pickle protocol versions 0 through 4.

To verify, you can send a test metric with a command like:

python -c 'import pickle, struct, os; payload = pickle.dumps([("metric",(1,2))],protocol=2); os.write(1, struct.pack("!L", len(payload)) + payload)' | nc localhost 2014

Replace protocol=2 to test other supported versions, to use protocol 3 or 4 you need to be running python 3.x

@DanCech DanCech requested a review from Dieterbe January 22, 2019 18:37
@Dieterbe Dieterbe changed the title update pickle payload prefix test to support pickle protocol versions 0, 1, 2, 3 & 4 support pickle protocol versions 0, 1, 2, 3 & 4 Feb 4, 2019
input/pickle.go Outdated Show resolved Hide resolved
@Dieterbe
Copy link
Contributor

Dieterbe commented Feb 6, 2019

I can confirmed that when doing carbon-relay-ng -> carbon-relay-ng using pickle, the following error

pickle.go: Unrecognized type []interface {} for item.

... is fixed

  1. by the 2nd commit
  2. if you isolate out only the receiver side fix from the 2nd commit
  3. if you isolate out only the sender side fix from the 2nd commit

so we were sending out []interface{} (which becomes a pickled array/list), which is the same as what some other tools do,
but we were only accepting pickled tuples (like carbon-relay.py sends, but not our own code itself)
inspired by buckytools (
https://github.com/jjneely/buckytools/blob/f3f6c69e2987035d3c0dd3fcbc24bf3afb2e9023/cmd/bucky-pickle-relay/main.go#L254) we are now more permissive on the receiver side, as well as send the data more cleanly as a tuple just like python code.

@Dieterbe Dieterbe changed the title support pickle protocol versions 0, 1, 2, 3 & 4 support pickle protocol versions 0, 1, 2, 3 & 4 + accept pickle arrays + send pickle tuples Feb 6, 2019
@Dieterbe Dieterbe merged commit 6f7fff5 into master Feb 6, 2019
@yanlibert
Copy link

This is exactly what I needed. I had some problem upgrading to the latest version in production due to some pickle data being dropped. It's all fine now. Thanks a lot for the good work!

@DanCech DanCech deleted the pickle-payload-prefix branch April 19, 2019 16:17
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

Successfully merging this pull request may close these issues.

[v0.11.0] pickle handler: invalid payload prefix
3 participants