-
Notifications
You must be signed in to change notification settings - Fork 299
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
Is it time to move to http/1.1 by default for outbound connections? #1121
Comments
@philippe44 you had done some important work to support http 1.1 a few years back (see eg. the line @PaulWebster is pointing to). I honestly don't understand what the impact might be moving all requests to 1.1 by default. Would you have any guess? I believe the above line isn't what is causing the issue. But the scanner would request 1.0 by default. If you changed it to request 1.1 instead it would pass the station you listed (I believe): diff --git a/Slim/Utils/Scanner/Remote.pm b/Slim/Utils/Scanner/Remote.pm
index 6fbad4da8..ee8f29e2c 100644
--- a/Slim/Utils/Scanner/Remote.pm
+++ b/Slim/Utils/Scanner/Remote.pm
@@ -203,6 +203,7 @@ sub scanURL {
# Connect to the remote URL and figure out what it is
my $request = HTTP::Request->new( GET => $url );
+ $request->protocol('HTTP/1.1');
main::DEBUGLOG && $log->is_debug && $log->debug("Scanning remote URL $url"); |
I came across another one that does not like HTTP/1.0 (recent addition to hisresaudio.online) I then changed Utils/Scanner/Remote.pm as earlier suggested to request 1.1 and it then gets a 200 OK. However, it stops almost immediately
I see that they use chunked encoding but I have seen other sources using chunked encoding with LMS and it seems to work. Same stream URL does play in vlc |
I'm sorry, this is nothing I'm familiar with... maybe @bpa-code has a better understanding? |
I only used started using HTTP 1.1 for HLS via a plugin lib module to have persistent connections. HTTP 1.1 has other additions besides persistent connection not sure of their status with LMS implementation. |
You'll get chunked encoding responses which won't be unpacked for you if
using AnyEvent::HTTP's want_steam_handle.
…On Sat, 22 Jun 2024, 16:40 bpa, ***@***.***> wrote:
I only used started using HTTP 1.1 for HLS via a plugin lib module to have
persistent connections.
Philippe implemented fully within LMS possibly for UPNP or bridge stuff.
HTTP 1.1 has other additions besides persistent connection not sure of
their status with LMS implementation.
If we enable HTTP 1.1 by default, what HTTP 1.1 features will be requested
by LMS or LMS plugins ?
If none then I think it is unlikely to be a problem as most sites have
implemented 1.1 as far LMS applications usage.
—
Reply to this email directly, view it on GitHub
<#1121 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAL3JV23Y5MYNW4ZUMAS453ZIWLGXAVCNFSM6AAAAABJMM2E36VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBUGA3TMOBZGI>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Not clear what point the AnyEvent::HTTP comment is addressing. Referring to initial comment - I think any HLS (i.e.m3u8) or DASH stream will expect to use HTTP 1.1 - HLS/DASH has a performance hit if TCP connection is not persistent - it is more significant to broadcasters/CDNs that to LMS users. Chunked encoding seems to be handled within LMS at certain places so I'm guessing changing default will not an issues for existing LMS application. If the default was changed to HTTP 1.1, should calls like SimpleAsynchHTTP have |
I was referring to the point that, when |
Are you sure ? When I made modifications to the LMS http code a long while ago for 1.1, I think I did what was necessary to handle chunked encoding and persistent connections, i.e not close when whole body has been received. The lower level code which is some Net::XXX AFAIR is in fact already handles mostly the chunk aggregation over Async. |
Great! Like I said, I had not actually checked current use. I'll shut up now. |
But I'm not 100% sure, that was a while ago ... |
Looking though LMS code - I think all the chunked support is as LMS as a recipient of a request (e.g. a JSONRPC or a UPNP request) requiring chunked encoding. The issues is about LMS making HTTP 1.1 requests. There is a comment in routine send_request in Slim::Networking::Async
I think we need to get clarification whether this comment is out of date now. I'm not sure whether LMS as a client supports sending HTTP 1.1. request asking for chunked encoding. The pertinent question may be whether LMS as a client needs to support chunked-encoding. About 2 yrs ago, I did a test version of my PlayHLS using LMS routines which essentially did
So they are HTTP 1.1 requests but with no headers to have chunked encoding. It worked OK probably because HLS chunks have definite length (i.e. not streams). I think the Paul's original request is similar. The expected replies are definite length so "stream" is not required. If there is no need to support "chunked encoding" in HTTP 1.1. GET request - then it is probably OK to enable HTTP 1.1 by default. |
When Andy commented out that line he left the following commit message:
I don't know how or if this is relevant. Just wanted to mention it. |
@PaulWebster asked me in the forum to add to this if my results were different. So I used above https://station.thecheese.co.nz/hls/the_cheese/stream_high.m3u8 link and curl 7.81.0 on Linux:
|
Update from TheCheese - they have heard of our problem and so have removed their block of HTTP/1.0 via Cloudflare. Also @Moonbase59 - I think, based on the headers shown, that your http/1.0 test used http/1.1 anyway. Perhaps that version of curl does not honour the command line request or determines that the target system does not say it accepts it so it moves to 1.1 |
Yeah, I wondered, because it said |
From what I read the discussion about TheCheese, the issue about not supporting HTTP 1.0 seemed to be tied into TLS 1.0 and 1.1 but disabling HTTP 1.0 is collateral damage not the reason itself. The dev's assumption was that old devices that use HTTP 1.0 probably only support TLS 1.0 and 1.1 and so since TLS 1.2 or higher is recommended for security,it is probably OK to limit connections to HTTP 1.1. That said, it is probably better that LMS support HTTP 1.1 and chunked encoding and so some investigation is needed. |
For completeness. On LMS 8.5.2 and the dev plugin can play the HLS/Flac stream https://station.thecheese.co.nz/hls/the_cheese/stream_high.m3u8 |
That URL works for me on LMS 9 with regular PlayHLSv2 - but since they removed the block on http/1.0 I was not surprised. Should I have been? |
LMS does an initial HTTP1.0 GET of the m3u8 file before handling over the PlayHLS based on MIME/suffix returned - if that initial GET is blocked then stream can't play even with PlayHLS own support of HTTP 1.1 PlayHLSV2 was developed for LMS 7.8/7.9 - has its own version of AnyEvent HTTP to support HTTP 1.1 as LMS 7.9 didn't support HTTP 1.1. HTTP 1.1. is used for persistent connection - the load on small device was high with HLS on HTTP 1.0 I think HLS Flac is only supported in fMPEG4. The Cheese "HLS" URL is a MPEG-2 stream and looks like to be AAC.
|
Just a note as already mentioned above The Cheese now allows HTTP/1.0 based on feedback and I am monitoring usage here. If you however wish to conduct testing then you're welcome to do so via a sister station who has a FLAC stream also (@PaulWebster) where I've just deployed the exact same rules that were on The Cheese (also blocking HTTP/1.0) and only allowing TLS 1.2/1.3 - h t t p s : / / station.huttcityfm.co.nz/listen/hutt_city_fm/flac Note: there is no station data for this (yet) as we're still working on migrating it over to newer infrastructure. |
Looking further into HTTP/1.1 and chunked encoding. AFAICT LMS does not use AnyEvent::HTTP - so any handling of chunked data is within LMS routines. There are two places where HTTP/1.0 request are sent by LMS. I did tests where I changed The LMS "HTTP 1.0" requests into HTTP 1.1 to observe effects. Slim/Player/Protocols/HTTP.pm is used to proxy request from a players GET when streaming audio. I think the chunked encoding can be handled by adding an appropriate "processor" on detection of the encoding header. Slim/Networking/Async/HTTP.pm is used for "other" HTTP request (e.g. SimpleAsyncHTTP) - I think the chunked encoding can be handled by updating the _http_read_body and associated routines. Currently a player playing "direct" uses a HTTP 1.0 request. Is there something that needs to be done ? Given that many stream already require https is it an issue at all ? LMS proxies players for "https" - should it also be done for HTTP 1.1 ? I'll try to update routines to explore "chunked" support. |
Occasionally (admittedly rarely) I have had issues when trying to play radio streams and have traced one problem to LMS using http/1.0
In one case the block is being done by Cloudflare .. with the response
"This website is using a security service to protect itself from online attacks. The action you just performed triggered the security solution. There are several actions that could trigger this block including submitting a certain word or phrase, a SQL command or malformed data."
To show in action .... try using curl to access
h t t p s : / / station.thecheese.co.nz/hls/the_cheese/stream_high.m3u8
then try again with --http1.0 added to the curl command
I override the version when fetching metadata for this station (although their metadata is currently broken) - but I do not get involved with the stream playing so that now fails since they changed their stream URL to the one above.
slimserver/Slim/Networking/Async/HTTP.pm
Line 305 in 8c75939
The text was updated successfully, but these errors were encountered: