rakshasa / rtorrent

rTorrent BitTorrent client
https://github.com/rakshasa/rtorrent/wiki
GNU General Public License v2.0
4.05k stars 412 forks source link

Bug with dual stack in CurlGet #1204

Open anthonyryan1 opened 1 year ago

anthonyryan1 commented 1 year ago

CC @rakshasa

I'm happy to submit a patch for this, but first I wanted to check which solution you would prefer.

curl_get.cc:95

We set CURL_IPRESOLVE_WHATEVER which doesn't specify a protocol. On a dual stack system with both IPv4 and IPv6, curl will generally prefer IPv6 these days. We also mark the connection as IPv4 without checking what we got.

  curl_easy_setopt(m_handle, CURLOPT_IPRESOLVE,      CURL_IPRESOLVE_WHATEVER);

  curl_easy_setopt(m_handle, CURLOPT_ENCODING,       "");

  m_ipv6 = false;

curl_stack.cc:141

We retry connections marked as IPv4 with IPv6.

    if (!(*itr)->is_using_ipv6()) {
      (*itr)->retry_ipv6();

curl_get.cc:121

Retry does correctly force IPv6.

  curl_easy_setopt(nhandle, CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V6);

This results in a situation where on a dual stack systems (getting more popular), rtorrent will sometimes announce on IPv6, fail and retry IPv6 again. Meaning that it never properly tries IPv4.

As for solutions, we could:

  1. Replace CURL_IPRESOLVE_WHATEVER with CURL_IPRESOLVE_V4, which will make it behave how the logic seems to intend, attempting IPv4 then falling back to IPv6.
  2. Flip the logic the so we explicitly try IPv6 first. Replace retry_ipv6() with retry_ipv4() on failure.
  3. If we could continue to use CURL_IPRESOLVE_WHATEVER and have curl let us know which protocol it chose. We could try the other one and not force an attempt order. Unfortunately I'm not aware of any way to check this from within the curl API.

The first solution is probably more compatible with things as they exist right now, since it's more likely for something to be IPv4 only than IPv6 only.

The second solution is much more future-proof, getting us ready for a world where eventually most people are IPv6.

I think my personal preference is towards the second solution, better to design for the future than the past in my opinion, but I would value your input.