Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

netcon update #588

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

winterheart
Copy link
Collaborator

Pull Request Type

  • GitHub Workflow changes
  • Documentation or Wiki changes
  • Build and Dependency changes
  • Runtime changes
    • Render changes
    • Audio changes
    • Input changes
    • Network changes
    • Other changes

Description

Replace old inetgetfile with new httpclient that allows download missions and other files via cpp-httplib library. That fixes thread safety errors in Descent3 Online netcon module. Reactivated ability download missing mission files from game browser.

Related Issues

Screenshots (if applicable)

Checklist

  • I have tested my changes locally and verified that they work as intended.
  • I have documented any new or modified functionality.
  • I have reviewed the changes to ensure they do not introduce any unnecessary complexity or duplicate code.
  • I understand that by submitting this pull request, I am agreeing to license my contributions under the project's license.

Additional Comments

@0xFADDAD
Copy link

A thought just occurred to me, plain HTTP is all but extinct, replaced with HTTPS. It might be a good idea to implement it at some point.

@winterheart
Copy link
Collaborator Author

httplib can handle HTTPS although on some platform this can be complicated - it uses OpenSSL, and Windows may has large dependency footprint.

@winterheart winterheart force-pushed the netcon-update branch 5 times, most recently from 0ae9d65 to 10b5945 Compare September 18, 2024 18:48
Descent3/mission_download.cpp Outdated Show resolved Hide resolved
Descent3/mission_download.cpp Outdated Show resolved Hide resolved
Descent3/mission_download.cpp Outdated Show resolved Hide resolved
Descent3/mission_download.cpp Outdated Show resolved Hide resolved
Descent3/mission_download.cpp Outdated Show resolved Hide resolved
Descent3/mission_download.cpp Show resolved Hide resolved
netcon/inetfile/httpclient.cpp Outdated Show resolved Hide resolved
@Lgt2x
Copy link
Member

Lgt2x commented Sep 18, 2024

httplib can handle HTTPS although on some platform this can be complicated - it uses OpenSSL, and Windows may has large dependency footprint.

I'm also scared of shipping OpenSSL but we may not cut it, doing plain HTTP is not really recommended anymore :/

Cleanup unused files, isolate submodule.
Inetgetfile has own HTTP parsing code which is pretty outdated. New HttpClient is based on external cpp-httplib library which provides up-to-date functionality for HTTP interactions.
Fixes memory corruption and provides more robust way to fetch and parse gameserver list. Now client working with list in memory instead of temporary file.
Rename strupr() to _strupr() to silence some MSVC compiler warnings.
Don't block on timeouts, failures etc. Rewritten D3::HttpClient to reflect changes.
@Lgt2x
Copy link
Member

Lgt2x commented Sep 24, 2024

How would I go about testing it? I'm entering Descent 3 online, join Descent Lobby, select one random server and click download, it always fails.

download

@winterheart
Copy link
Collaborator Author

Log should reveal real error. Specifically, www.planetdescent.com leads to IGN site instead of ZIP file, and zip-extractor fails to extract downloaded html-page. I've tested on www.descentforum.net and www.dfiles.de, usually they should works.

@Lgt2x
Copy link
Member

Lgt2x commented Sep 24, 2024

When testing on server DF.NET Tsetsefly3 / mission Coliseum, selecting DescentForum.net link, a crash happens:

log

2024-09-24 19:31:12.419 DEBUG [87794] [msn_CheckGetMission@610] Downloading missions file from http://www.descentforum.net/missions/coliseum.zip
2024-09-24 19:31:12.420 DEBUG [87794] [msn_DownloadWithStatus@324] We're downloading a zip file!!!

Call stack

[Unknown/Just-In-Time compiled code] (Unknown Source:0)
std::function<void(int)>::function(std::function<void(int)> * this, const std::function<void(int)> & __x) (/usr/include/c++/14.2.1/bits/std_function.h:391)
httplib::ClientImpl::create_client_socket(const httplib::ClientImpl * this, httplib::Error & error) (/usr/include/httplib.h:7267)
httplib::ClientImpl::create_and_connect_socket(httplib::ClientImpl * this, httplib::ClientImpl::Socket & socket, httplib::Error & error) (/usr/include/httplib.h:7274)
httplib::ClientImpl::send_(httplib::ClientImpl * this, httplib::Request & req, httplib::Response & res, httplib::Error & error) (/usr/include/httplib.h:7382)
httplib::ClientImpl::send(httplib::ClientImpl * this, httplib::Request & req, httplib::Response & res, httplib::Error & error) (/usr/include/httplib.h:7350)
httplib::ClientImpl::send_(httplib::ClientImpl * this, httplib::Request && req) (/usr/include/httplib.h:7455)
httplib::ClientImpl::Get(httplib::ClientImpl * this, const std::string & path, const httplib::Headers & headers, httplib::ResponseHandler response_handler, httplib::ContentReceiver content_receiver, httplib::Progress progress) (/usr/include/httplib.h:8068)
httplib::ClientImpl::Get(httplib::ClientImpl * this, const std::string & path, httplib::ContentReceiver content_receiver, httplib::Progress progress) (/usr/include/httplib.h:8014)
httplib::Client::Get(httplib::Client * this, const std::string & path, httplib::ContentReceiver content_receiver, httplib::Progress progress) (/usr/include/httplib.h:9641)
D3::HttpClient::Get(D3::HttpClient * this, const std::string & URIPath, const httplib::ContentReceiver & content_receiver, const httplib::Progress & progress) (/home/louis/dev/Descent3/netcon/inetfile/httpclient.cpp:43)
std::__invoke_impl<httplib::Result, httplib::Result (D3::HttpClient::*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<bool (char const*, unsigned long)> const&, std::function<bool (unsigned long, unsigned long)> const&), D3::HttpClient*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_0, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_1>(std::__invoke_memfun_deref, httplib::Result (D3::HttpClient::*&&)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<bool (char const*, unsigned long)> const&, std::function<bool (unsigned long, unsigned long)> const&), D3::HttpClient*&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_0&&, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_1&&)(httplib::Result (D3::HttpClient::*&&)(D3::HttpClient * const, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > &, const std::function<bool(char const*, unsigned long)> &, const std::function<bool(unsigned long, unsigned long)> &) __f, D3::HttpClient *&& __t, class {...} && __args, class {...} && __args, class {...} && __args) (/usr/include/c++/14.2.1/bits/invoke.h:74)
std::__invoke<httplib::Result (D3::HttpClient::*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<bool (char const*, unsigned long)> const&, std::function<bool (unsigned long, unsigned long)> const&), D3::HttpClient*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_0, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_1>(httplib::Result (D3::HttpClient::*&&)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<bool (char const*, unsigned long)> const&, std::function<bool (unsigned long, unsigned long)> const&), D3::HttpClient*&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_0&&, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_1&&)(httplib::Result (D3::HttpClient::*&&)(D3::HttpClient * const, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > &, const std::function<bool(char const*, unsigned long)> &, const std::function<bool(unsigned long, unsigned long)> &) __fn, class {...} && __args, class {...} && __args, class {...} && __args, class {...} && __args) (/usr/include/c++/14.2.1/bits/invoke.h:96)
std::thread::_Invoker<std::tuple<httplib::Result (D3::HttpClient::*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<bool(char const*, unsigned long)> const&, std::function<bool(unsigned long, unsigned long)> const&), D3::HttpClient*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_0, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_1> >::_M_invoke<0ul, 1ul, 2ul, 3ul, 4ul>(std::thread::_Invoker<std::tuple<httplib::Result (D3::HttpClient::*)(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > &, const std::function<bool (const char *, unsigned long)> &, const std::function<bool (unsigned long, unsigned long)> &), D3::HttpClient *, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, (lambda at /home/louis/dev/Descent3/Descent3/mission_download.cpp:404:7), (lambda at /home/louis/dev/Descent3/Descent3/mission_download.cpp:409:7)> > * this) (/usr/include/c++/14.2.1/bits/std_thread.h:301)
std::thread::_Invoker<std::tuple<httplib::Result (D3::HttpClient::*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<bool(char const*, unsigned long)> const&, std::function<bool(unsigned long, unsigned long)> const&), D3::HttpClient*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_0, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_1> >::operator()(std::thread::_Invoker<std::tuple<httplib::Result (D3::HttpClient::*)(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > &, const std::function<bool (const char *, unsigned long)> &, const std::function<bool (unsigned long, unsigned long)> &), D3::HttpClient *, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, (lambda at /home/louis/dev/Descent3/Descent3/mission_download.cpp:404:7), (lambda at /home/louis/dev/Descent3/Descent3/mission_download.cpp:409:7)> > * this) (/usr/include/c++/14.2.1/bits/std_thread.h:308)
std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<httplib::Result>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<httplib::Result (D3::HttpClient::*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<bool(char const*, unsigned long)> const&, std::function<bool(unsigned long, unsigned long)> const&), D3::HttpClient*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_0, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_1> >, httplib::Result>::operator()(const std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<httplib::Result>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<httplib::Result (D3::HttpClient::*)(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > &, const std::function<bool (const char *, unsigned long)> &, const std::function<bool (unsigned long, unsigned long)> &), D3::HttpClient *, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, (lambda at /home/louis/dev/Descent3/Descent3/mission_download.cpp:404:7), (lambda at /home/louis/dev/Descent3/Descent3/mission_download.cpp:409:7)> >, httplib::Result> * this) (/usr/include/c++/14.2.1/future:1416)
std::__invoke_impl<std::unique_ptr<std::__future_base::_Result<httplib::Result>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<httplib::Result>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<httplib::Result (D3::HttpClient::*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<bool(char const*, unsigned long)> const&, std::function<bool(unsigned long, unsigned long)> const&), D3::HttpClient*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_0, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_1> >, httplib::Result>&>(std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<httplib::Result>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<httplib::Result (D3::HttpClient::*)(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > &, const std::function<bool (const char *, unsigned long)> &, const std::function<bool (unsigned long, unsigned long)> &), D3::HttpClient *, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, (lambda at /home/louis/dev/Descent3/Descent3/mission_download.cpp:404:7), (lambda at /home/louis/dev/Descent3/Descent3/mission_download.cpp:409:7)> >, httplib::Result> & __f) (/usr/include/c++/14.2.1/bits/invoke.h:61)
std::__invoke_r<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<httplib::Result>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<httplib::Result (D3::HttpClient::*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<bool(char const*, unsigned long)> const&, std::function<bool(unsigned long, unsigned long)> const&), D3::HttpClient*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_0, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_1> >, httplib::Result>&>(std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<httplib::Result>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<httplib::Result (D3::HttpClient::*)(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > &, const std::function<bool (const char *, unsigned long)> &, const std::function<bool (unsigned long, unsigned long)> &), D3::HttpClient *, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, (lambda at /home/louis/dev/Descent3/Descent3/mission_download.cpp:404:7), (lambda at /home/louis/dev/Descent3/Descent3/mission_download.cpp:409:7)> >, httplib::Result> & __fn) (/usr/include/c++/14.2.1/bits/invoke.h:114)
std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter>(), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<httplib::Result>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<httplib::Result (D3::HttpClient::*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<bool(char const*, unsigned long)> const&, std::function<bool(unsigned long, unsigned long)> const&), D3::HttpClient*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_0, msn_DownloadWithStatus(char const*, std::filesystem::__cxx11::path const&)::$_1> >, httplib::Result> >::_M_invoke(const std::_Any_data & __functor) (/usr/include/c++/14.2.1/bits/std_function.h:290)
std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter>()>::operator()(const std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter>()> * this) (/usr/include/c++/14.2.1/bits/std_function.h:591)

In create_client_socket error is httplib::Error::Success

VCPKG build on Arch Linux

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants