mldonkey icon indicating copy to clipboard operation
mldonkey copied to clipboard

Reimplemented http_client using the ocurl library

Open carlonluca opened this issue 11 months ago • 1 comments

This commit reimplements the http_client module by using the ocurl library. This should add support for the https protocol.

carlonluca avatar Feb 09 '25 01:02 carlonluca

The implementation is still a work in progress, which is the reason why it is a bit too verbose. This is the first patch needed to fix #119 and #41. Comments are welcome.

carlonluca avatar Feb 09 '25 01:02 carlonluca

Also, I suppose GitHub workflow now needs libcurl-ocaml-dev installed too.

xOneca avatar Jun 23 '25 12:06 xOneca

So, here I was thinking this was the only HTTP client implementation in MLDonkey, and in fact there are two! :confused:

As I can see, the HTTP client in this PR is used only for:

  • WebInfos: tested both HTTP and HTTPS and works perfectly! :100:
  • Torrent trackers: could not find (yet) an HTTPS tracker, but HTTP works!

And dllink <url> also seems to use it (to no avail):

2025/06/25 16:45:57 [dCmd] execute command "dllink" "https://cdimage.debian.org/debian-cd/current/amd64/bt-cd/debian-12.11.0-amd64-netinst.iso.torrent"
2025/06/25 16:45:57 [HTTPcl] whead
2025/06/25 16:45:57 [HTTPcl] wget_string https://cdimage.debian.org/debian-cd/current/amd64/bt-cd/debian-12.11.0-amd64-netinst.iso.torrent
2025/06/25 16:45:58 [HTTPcl] HTTP success: https://cdimage.debian.org/debian-cd/current/amd64/bt-cd/debian-12.11.0-amd64-netinst.iso.torrent
2025/06/25 16:45:58 [BT] Not_found and url has non valid torrent extension: Date: Wed, 25 Jun 2025 16:45:58 GMT
Server: Apache/2.4.63 (Unix)
Last-Modified: Sat, 17 May 2025 11:47:38 GMT
Accept-Ranges: bytes
Content-Length: 54073
Age: 3184
Content-Type: application/x-bittorrent

It seems to download the torrent file, but then fails to detect that it is, in fact, a torrent file and the torrent is not enqueued for download. Tried both HTTP and HTTPS. :disappointed:

Seems to be parsing headers as the name of the file? Maybe searching the name in the headers? :thinking:

xOneca avatar Jun 25 '25 17:06 xOneca

Hello, yes, there are two different implementations of HTTP client. I discussed this point somewhere in the issues. From what I can remember, one implementation is only used for FileTP, and I said I won't work on that. I think I tested HTTPS trackers with an Ubuntu torrent, and it worked.

Much time has passed since I sent this PR, but I think torrent files are downloaded with the new implementation. In fact, I tested your link and also this one https://releases.ubuntu.com/25.04/ubuntu-25.04-desktop-amd64.iso.torrent. Both started immediately.

I remember I had to fix multiple bugs related to this problem. Can you confirm you are not able to download that torrent you provided?

carlonluca avatar Jun 25 '25 20:06 carlonluca

Ok, I suspect I missed some commits in this PR. I probably restricted this PR to the HTTPS client itself. This may fix the issue you are seeing: https://github.com/carlonluca/mldonkey/commit/d4592d757032541ebd2d610a751706cf0acc37a7. My fork includes this, but I can't find it in this PR.

Unfortunately my fork has diverged quite a bit. I'll have to go through everything again and see if something else is missing.

In the meantime, thanks for your review. I'll work on what is missing.

carlonluca avatar Jun 25 '25 20:06 carlonluca

Ok, sorry, it seems you may need more than this PR to get proper HTTPS for bittorrent support. This is what I have in my fork: https://github.com/carlonluca/mldonkey/commits/dev/. You probably want to use that if you want bittorrent to work properly. This PR is strictly for the client.

carlonluca avatar Jun 25 '25 21:06 carlonluca

I moved my fork to dune and ocaml5, making this PR pretty outdated (more info here: https://bugfreeblog.duckdns.org/2025/08/mldonkey-ocaml5.html). I will not update this further so I close it.

carlonluca avatar Aug 16 '25 12:08 carlonluca