Initial HTTPS support via Naett (partial platform support) #17744
Conversation
|
I don't understand why the check is failing. Locally on my linux machine, it now avoids building naett.c (due to the curl dependency on linux) just fine. |
|
|
|
@anr2me yes, I know. but naett.c isn't supposed to be building at all on linux yet, that's the problem. Somehow the logic to prevent it works on my laptop but not on CI. |
hrydgard
changed the title
There was a problem hiding this comment.
Outside of cancelling and progress issues, it might be nice even to use this for all requests once ready. In theory, using WinHTTP or etc. will use system proxy settings, which might be better.
The web debugger can't use https easily on the server side (which naett and probably winhttp don't even support), but in theory even the HTTPFileLoader could use naett. The https problem there is about certificates, in theory of course it could serve using a self-signed certificate but that would cause people to get security alerts (or in most cases, just errors they can't easily skip) trying to use it.
-[Unknown]
|
|
||
| const char *methodStr = method_ == RequestMethod::GET ? "GET" : "POST"; | ||
| req_ = naettRequest_va(url_.c_str(), !postMime_.empty() ? 3 : 2, naettMethod(methodStr), naettHeader("accept", "application/json, text/*; q=0.9, */*; q=0.8"), naettHeader("Content-Type", postMime_.c_str())); | ||
| res_ = naettMake(req_); |
There was a problem hiding this comment.
Seems like this is missing any timeout setting, which naett seems to support.
-[Unknown]
There was a problem hiding this comment.
Added (though will make more configurable in a followup with a RequestDesc struct
There was a problem hiding this comment.
Yeah, there's SetDataTimeout() currently, which I guess Download doesn't currently call.
-[Unknown]
|
I've patched naett with two PRs: erkkah/naett#18 and erkkah/naett#17 , temporarily, to achieve progress reporting and user agent setting. |
|
I'm gonna get this in as-is, so it gets tested properly ASAP. I'll move naett back to upstream once my PRs are merged. |
|
Since Naett can't set user-agent before, i guess it doesn't support custom headers too, right? (since user-agent is a part of header too) Edit: nevermind, naett's example did shows the ability to add custom header(s), so i guess it won't be an issue as long i use naett directly. |
Naett is a little C wrapper library on top of multiple OS-specific HTTPS request implementations. By far the most lightweight dependency we can take to support these.
Might switch to something more independent later, see #17583 , but this looks like it will suffice for now to avoid shipping an Android release without RetroAchievements https support.
This enables naett for HTTPS on:
The importance of this is because RetroAchievements doesn't have a policy on how long they'll keep http support alive, and new clients are expected to understand https. So I don't want to ship official binaries on Win/Android/Mac that don't support https.
This doesn't yet enable naett on Linux since it requires curl with openssl support, and I'm a bit unclear about how to properly set up that dependency from cmake. And if we're gonna use openssl anyway (which we really, really don't want to do on Windows) we could wrap it around our own http client instead, though will be some extra work.
On platforms not supported by naett, we'll have to avoid https, still. Added a temporary HTTPS_NOT_AVAILABLE build flag for this, so that the upcoming PRs that enable HTTPS for RA and the store can check it. Initially, I only plan to use HTTPS selectively for RetroAchievements, and for the homebrew store too mostly as a test case.
Missing before merge:
Next steps after this:
#17750 should be merged before this.