Next PRs for review:
See this report by @fjahr. A few of these have already been addressed, like #29904
Moreover, the new non-released code breaks our fuzzing tests. See: #30096.
I’m not so sure that’s relevant. I think the conclusion in that issue was that vcpkg should probably stop shipping unstable/unreleased code, and I don’t think we care about fuzzing unreleased versions of dependencies. It’s less suprising that code which is still in development / might have API or other breaking changes we haven’t accounted for might not work correctly.
Concept ACK on removing libevent.
In addition to the motivation from the OP, I would add that libevent is non-intuitive and difficult to work with (at least for me, this could be subjective). It was related to a remote crash via RPC: https://github.com/bitcoin/bitcoin/pull/27468
I have been working on the removal of libevent from torcontrol but I still see some errors that I need to debug. I hope I can open a PR soon.
:heart:
poke me for review
Agree
On Thu, Nov 7, 2024, 2:46 AM Vasil Dimov @.***> wrote:
I have been working on the removal of libevent from torcontrol but I still see some errors that I need to debug. I hope I can open a PR soon.
❤️
poke me for review
— Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/issues/31194#issuecomment-2461527876, or unsubscribe https://github.com/notifications/unsubscribe-auth/BDSRHFZPVAHZK2F7J7W2T5LZ7MLGLAVCNFSM6AAAAABQ6RHYAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRRGUZDOOBXGY . You are receiving this because you are subscribed to this thread.Message ID: @.***>
I found another edge case in libevent regarding URI handling, it does not decode %XX characters before parsing.
This is in libevent’s evhttp_uri_parse() which is called by Bitcoin’s GetQueryParameterFromUri()
Example, the URI:
/rest/endpoint/someresource.json%3Fp1%3Dv1%26p2%3D100%25
decodes to /rest/endpoint/someresource.json?p1=v1&p2=100%
It will be interpreted by libevent (and therefore bitcoin core, currently) as one long path with no query parameters.
Both Python’s urlparse() and JavaScript’s decodeURIComponent() will decode the %XX characters before parsing and identify the query, with two parameters, as separate from the path
I’ve checked that and I see where the problem is…
If you run the following it works correctly (using/ changing src/test/httpserver_tests.cpp):
0 uri = "/rest/endpoint/someresource.json?p1=v1%20&p1=v2";
1 BOOST_CHECK_EQUAL(GetQueryParameterFromUri(uri.c_str(), "p1").value(), "v1 ");
0./build/src/test/test_bitcoin --log_level=all --run_test=httpserver_tests
1...
2./test/httpserver_tests.cpp(52): info: check GetQueryParameterFromUri(uri.c_str(), "p1").value() == "v1 " has passed
3...
So evhttp_uri_parse() works as expected(?) as soon the “encoded” char is in the variable’s value (e.g. the following won’t work: “/rest/endpoint/someresource.json%3Fp1=v1%20&p1=v2”).
For the case where the “encoded” chars are among the path, we’d need to run the evhttp_uridecode():
0 uri = "/rest/endpoint/someresource.json%3Fp1%3Dv1%26p2%3D100%2525";
1 char *decoded_path = evhttp_uridecode(uri.c_str(), 0, NULL); // Use c_str() here
2 BOOST_REQUIRE(decoded_path != nullptr); // Ensure decoding succeeded
3
4 std::cout << "Decoded path: " << decoded_path << std::endl;
5
6 // Test query parameter extraction
7 BOOST_CHECK_EQUAL(GetQueryParameterFromUri(decoded_path, "p2").value(), "100%");
8
9 free(decoded_path); // Don't forget to free the decoded string
But as you can see the “encoded” path should be “/rest/endpoint/someresource.json%3Fp1%3Dv1%26p2%3D100%2525” otherwise the libevent parse will fail because as in your example the “/rest/endpoint/someresource.json?p1=v1&p2=100%” is invalid (RFC 3986:A % character must always be followed by two valid hexadecimal digits (0–9, A–F, a–f) that represent a byte value in hexadecimal notation). evhttp_uridecode() doesn’t check if the encoded char is within the path or in the variable’s value so perhaps we need to handle this special use case somehow.
@pablomartin4btc thanks, I added this test to my branch:
0
1 // Multiple parameters, some characters encoded
2 uri = "/rest/endpoint/someresource.json?p1=v1%20&p2=100%25";
3 BOOST_CHECK_EQUAL(GetQueryParameterFromUri(uri.c_str(), "p1").value(), "v1 ");
4 BOOST_CHECK_EQUAL(GetQueryParameterFromUri(uri.c_str(), "p2").value(), "100%");
You’re totally right, libevent decodes %XX but only after it has parsed the URI. Meaning the special characters ? & and = (probably also #) can’t be encoded the way we’re currently using libevent.