Use libsoup3 over curl #196
Conversation
so, somehow this error is being raised. My best guess is because |
LaserEyess
force-pushed
the
libsoup
branch
2 times, most recently
from
30d547b
to
d809bea
Compare
LaserEyess
force-pushed
the
libsoup
branch
10 times, most recently
from
bf83496
to
80bc17a
Compare
|
Forgot about SSL lol |
LaserEyess
force-pushed
the
libsoup
branch
3 times, most recently
from
acb7faf
to
17c9d00
Compare
|
I don't have a ton to say, honestly this looks excellent. I think you've done really well with all the error handling. |
|
I appreciate the review, I'm glad my first attempt at using libsoup didn't end in disaster. Unfortunately, unconditionally calling the callback like this code now does is wrong. I think I'm missing how I need to re-add that logic in a more intelligent way and then do more testing with ASAN since I seem to be getting a lot of UAF bugs. |
LaserEyess
force-pushed
the
libsoup
branch
2 times, most recently
from
f574c7e
to
a0c058a
Compare
|
Saving this for my own reference later: When using There is the potential to switch back to |
LaserEyess
force-pushed
the
libsoup
branch
2 times, most recently
from
47dbe8a
to
7995e5c
Compare
Previously it used a special #define to indicate that something had gone wrong, which didn't really seem practical. So, instead return a boolean for success/failure and explicitly pass an error message on what the failure was.
libsoup is a gobject based API with async that integrates well into the glib mainloop. The current curl threadpool implementation has a high amount of latency and is somewhat complex, so this simplifies a lot of code as well as adding a gobject based library for further extension. The changes can be summarized as the following 1. remove the threadpool, and curl handling 2. Vastly simplify trg_request and make it private, as well as removing the ability to do anything but handle JSON RPC 3. Put a SoupSession into TrgClient and allow that to handle all connections to the daemon 4. Arrange a series of callbacks for async RPC as well as JSON loading of results (see comment in src/trg-client.c) 5. Set up new error handling for bad requests 6. Clean up the rest of the code base to work with the above changes. Since this commit is very complex, even though it isn't 1:1 feature complete with the old curl implementation, other things will be added as separate commits for easier reviewing. This commit additionally bumps glib to 2.70 for GUri usage Co-Authored-By: Patrick Griffis <pgriffis@igalia.com>
This function existed previously but was unused, now it is more useful and flexible. Some slight code de-duplication is possible
This uses the SoupLogger functionality to add logging support to RPC calls if $TRG_CLIENT_DEBUG is set. This effect is also documented inside of the man page.
LaserEyess
force-pushed
the
libsoup
branch
2 times, most recently
from
719b95a
to
8c6d499
Compare
| priv->username = trg_prefs_get_string(prefs, TRG_PREFS_KEY_USERNAME, TRG_PREFS_CONNECTION); | ||
| priv->url = g_uri_parse(uri_str, G_URI_FLAGS_NONE, &uri_err); | ||
| if (uri_err) { | ||
| g_mutex_unlock(&priv->configMutex); |
There was a problem hiding this comment.
You might be interested in the GMutexLocker API. Nothing wrong here but I find it nicer to use.
There was a problem hiding this comment.
I think it should be possible to remove all the locking since we can restrict TrgPrefs to a single thread. That's for a later time though.
This should be working, minus the TODOs below:
Fix RSS stuff (just add a newNopeSoupSessionfor now, I guess)JsonObjectbeing NULLTrgMainWindowSoupLoggermaybe for debugging headers/connections?Make sure proxies work (they should work transparently?)latercloses #147 (I think)