use http1.1 #2177
use http1.1 #2177
Conversation
|
I'm not sure what the deal with that check failure is, except that it's for gcc5 which is...um...old. Do you even need to support gcc5? In any case, all I see is which for all I know could just be the OOM killer or something like that. I'll see if I can get gcc5 on my machine & replicate. FWIW, it seems kind of odd that you would run such a massive test suite in response to a PR from an internet rando (don't people find projects that do this & inject bitcoin miners or something?), but it's your project. |
|
nm, I now see it's a docker thing that failed; reproing |
|
As I expected I couldn't repro it (probably some kind of resource limit like OOM killer; makes sense given the size of the job). I only tried as far as running though |
|
I've also encountered this with wordpress blogs (turanszkij/WickedEngine#557). |
|
Sorry for the late response! I'm down with COVID and don't have energy to investigate this fully right now, but I ran a quick check with If this is indeed dependent on the curl version, then a better solution IMHO would be to only downgrade to HTTP/1.1 if Newsboat is linked against an old version. This can be done with an |
I think there's a decent chance this is dependent on the curl version, although it would be worth checking your |
|
Actually, if I run curl from a So it's definitely related to the curl version |
|
Also it works fine on I'm thinking it must be related to the nghttp2 version, not the curl version, but idk |
|
I also built curl 7.80 from git, which also failed. Since it worked fine from alpine, I assume it must be one of the libraries. |
|
Built curl locally with tip of curl and nghttp2 at v1.46.0 and it works: |
|
It also works locally with tip of both curl and nghttp2. Basically I don't think we can fix this with the preprocessor and curl version, since curl is just dynamically loading nghttp2, and newsboat doesn't even know nghttp2 exists. |
|
So it's almost certainly this same issue:
The best solution for me is to switch to using the tip of nghttp2 & curl. It's actually necessary to use the tip of curl, since curl 7.85.0 won't attempt to use the fix introduced in the tip of nghttp2. @Minoru if you want to fix this by checking the libcurl version, one way might be to check for libcurl version >= 7.86.0 (which doesn't actually exist yet), since that will fix it. I don't think any other libcurl version pin will help, since they can't exclude nghttp2 versions. |
|
@Minoru following up with curl & nghttp2 projects, I have determined that this issue is likely to be resolved in released versions in about 43 days, after which it is unlikely that this issue will occur for any but users of the shittiest package managers. Since I was able to mitigate this myself, and many other users will never hit it at all due to being behind the upstream release versions, I'm pretty sure we don't want to make the change in this PR just to address this specific issue. However, it might be worth asking whether we want to do it anyway to avoid similar issues in the future. http2 is still under active development, and may well have more new revisions which will then be implemented incorrectly by server developers and lead to more issues like this in the future. RFC 9113, the root of this issue, was published in June. No doubt the same server developers are hard at work misimplementing whatever IETF has published since then. Do we actually gain anything by using http2? Are there any actual servers publishing rss docs which are http2-only? This seems unlikely; afaik http2 only is extremely rare to begin with, and I'm almost positive the overwhelmingly most common way to use http2 at all is via ALPN after initially connecting with http1.1. I'm not 100% sure using http2 without ALPN is possible. Actually it might be better to change this from the equivalent of passing |
|
Thanks for digging up all the details @dradetsky! Looks like the only thing left to me here is to decide what to do with all this stuff :) I agree that HTTP/2 doesn't bring much to the RSS/Atom table. The only thing I can think of is connection coalescing, which probably helps with sites that are hosted on github.io, wordpress.com, blogger.com and similar platforms; reusing a single TLS connection there should bring a noticeable improvements to reload times, especially for the common case of 304 Not Modified. However, I feel uneasy dropping a protocol just because there could be more bugs in its implementations. I'm inclined to keep it and see if it really becomes a source of problems. We already have an escape hatch in the form of |
I attempted to add
https://perell.com/feed/to my feedlist. Attempted to sync this feed producedSince newsboat uses libcurl, I checked with curl (7.85.0) and sure enough:
Some poking around curl's issues suggests that this is likely due to an issue with the server not implementing http2 correctly. In any case, even if it isn't, fixing curl is clearly outside the scope of newsboat. It's possible that another option change could fix this, but I don't know what.
When we check curl's debug output, we see
In other words, the server claims to implement http2, but apparently does not (according to curl). I can open the url in a browser, and my browser reports that it's using h2, but that doesn't mean much; browsers probably do all kinds of sloppy protocol implementation handling.
Anyhow, I assume that since we don't currently specify a http version for libcurl, it just uses the highest version advertised. So it tries http2 and fails. AFAIK, curl doesn't have any kind of try-http2-and-if-that-fails-try-http1.1-etc option. newsboat could implement this kind of fallback logic itself, but this sounds like a lot of work. I'm not going to do it.
I could try to tell this Perell character to fix his web server, but I assume that this issue will occur elsewhere. Enough people seem to think RSS is obsolete that they may not be willing to devote a lot of effort to fixing an issue they don't really understand or may not control (in the case of hosted blogs).
So we need some way to force http1.1. This could be some kind of per-feed config, or just a blanket change for everything like I've done here. Which would probably be better but also more work and would require design decisions and I have no idea what a good design would be.
To the best of my knowledge, http2 is basically a browser-specific optimization. Like, it's better when you want to fetch 20 different documents from the same server (e.g. 19 css/js files and the html doc), but doesn't really make a difference for the fetch-one-rss-feed case. But I could be wrong about this.
Just as I was finishing this up, I identified another 11 urls in my feed that were experiencing this issue. So it's definitely not this one server. They mostly appear to originate from wordpress.com, although perell.com is coming from pressable. I believe in all cases the server identifies itself as nginx. In any case, it's sufficiently widespread that just telling people to fix their blogs seems like it won't work.