Implement a K/V parser library

The key/value format K=V is used extensively in the Tor code and seems many places uses their own way to parse that.

This ticket is for implementing a library to parse such K/V construction. There is a world where we might require V to be quoted or not so I'll let this challenge to the implementer(s).

This ticket comes from the work done in #28179 (moved) that implements messages from the PT to the Tor process.

changed milestone to %Tor: 0.4.0.x-final

added actualpoints::.3 component::core tor/tor milestone::Tor: 0.4.0.x-final owner::nickm parent::25502 priority::medium resolution::implemented reviewer::ahf reviewer::dgoulet severity::normal status::closed type::enhancement labels

Branch ticket28755 has my first take on this. PR at https://github.com/torproject/tor/pull/568 . More testing and/or fuzzing might be a good idea here.

Trac:
Status: assigned to needs_review
Actualpoints: N/A to .3

Trac:
Reviewer: N/A to ahf, dgoulet

We could test this branch by moving all our K=V parsing over to it, and running the unit and integration tests.

Looks very good for our use case. A fuzz function might be a good idea to get in.

Good call. I found a couple of bugs while fuzzing.

I've made a new ticket, "ticket28755_v2", including a fuzzer. PR at https://github.com/torproject/tor/pull/577

The initial implementation is a bit different, because the fuzzer found a bunch of problems, most of them related to eat_whitespace() not doing what I expected, and a few of them related to ambiguity involving weird standalone values like "=foo" or "".

The fuzzing corpus is in a "strops" branch in my private fuzzing-corpora repo; I'll merge that when you like this.

Provided a review. Majority of comments are about leftovers in the code. The more important thing is about a test case I tried which failed.

(Oh also... I don't think this is related but Travis fails on Stem test and NSS... ?)

Trac:
Status: needs_review to needs_revision

Okay, I think the branch is better now. The leftovers are gone, it passes the tests you gave, and still passes the fuzzer.

Trac:
Status: needs_revision to needs_review

Replying to nickm:

Okay, I think the branch is better now. The leftovers are gone, it passes the tests you gave, and still passes the fuzzer.

lgtm;

But before merging, can you do a fixup that adds the test cases I added on the ticket that caught AB=?

Trac:
Status: needs_review to merge_ready

Added those in a0a3694fd16660911756eb1d38c9c0efc4a35334.

Squashed and merged to master!

Trac:
Status: merge_ready to closed
Resolution: N/A to implemented

closed

added 2h 24m of time spent

moved to tpo/core/tor#28755 (closed)