karlheyes / icecast-kh

KH branch of icecast
GNU General Public License v2.0
299 stars 107 forks source link

Issue when requesting playlist using http range request #134

Open friedlsa opened 8 years ago

friedlsa commented 8 years ago

Hello, We have playlists (m3u) which are meant to be served by the static file serving mechanism. When you try to get this playlists using an range request, we get an response of the size of the playlist but the content is all 0xff. When the request is done without the range request, it works fine.

You can test that using curl. Without range request:

$ curl -v -s http://stream.srg-ssr.ch/drs1/mp3_128.m3u

  • Trying 193.218.103.243...
  • Connected to stream.srg-ssr.ch (193.218.103.243) port 80 (#0) GET /drs1/mp3128.m3u HTTP/1.1 Host: stream.srg-ssr.ch User-Agent: curl/7.43.0 Accept: /_

< HTTP/1.1 200 OK < Content-Length: 80 < Content-Type: audio/x-mpegurl < Server: Icecast 2.4.0-kh3 < Cache-Control: no-cache, no-store < Pragma: no-cache < Access-Control-Allow-Origin: * < Access-Control-Allow-Headers: Origin, Accept, X-Requested-With, Content-Type < Access-Control-Allow-Methods: GET, OPTIONS, HEAD < Connection: Keep-Alive < Expires: Mon, 26 Jul 1997 05:00:00 GMT < http://stream.srg-ssr.ch/m/drs1/mp3_128 http://stream.srg-ssr.ch/m/drs1/mp3_128

  • Connection #0 to host stream.srg-ssr.ch left intact

With range request:

$ curl -v -s -r 0- http://stream.srg-ssr.ch/drs1/mp3_128.m3u

  • Trying 193.218.104.243...
  • Connected to stream.srg-ssr.ch (193.218.104.243) port 80 (#0) GET /drs1/mp3128.m3u HTTP/1.1 Host: stream.srg-ssr.ch Range: bytes=0- User-Agent: curl/7.43.0 Accept: /_

< HTTP/1.1 206 Partial Content < Content-Type: audio/x-mpegurl < Accept-Ranges: bytes < Content-Length: 80 < Content-Range: bytes 0-79/80 < Server: Icecast 2.4.0-kh3 < Cache-Control: no-cache, no-store < Pragma: no-cache < Access-Control-Allow-Origin: * < Access-Control-Allow-Headers: Origin, Accept, X-Requested-With, Content-Type < Access-Control-Allow-Methods: GET, OPTIONS, HEAD < Connection: Keep-Alive < Expires: Mon, 26 Jul 1997 05:00:00 GMT <

  • Connection #0 to host stream.srg-ssr.ch left intact ��������������������������������������������������������������������������������

Many players including vlc and devices seem to make a range request.

I haven't found a config option or another answer on google and elsewhere.

We are using icecast-2.4.0kh3 on CentOS 7.

Our icecast config from one of the relays: icecast.txt

Best regards, Sam

onitake commented 8 years ago

Teardown of the problem:

  1. format_general_headers() in format.c allocates refbuf no. 1 and stores the HTTP response headers in it
  2. If the request is ranged, format_general_headers() allocates refbuf no. 2, range-length bytes long and initializes it to 0xff, then attaches it to refbuf no. 1.
  3. fserve_setup_client_fb() in fserve.c opens the file to serve
  4. fserve_setup_client_fb() calls prefile_send() (indirectly)
  5. prefile_send() in fserve.c sends the first refbuf containing the header
  6. prefile_send() sends the second refbuf and bails out with an error

As far as I can tell, there are two problems here:

In step 2, a buffer for the ranged content is allocated, but that buffer is never filled with the actual data. Filling this buffer directly in format_general_headers() is not possible, because the file isn't open yet at this point.

The second problem is the complicated logic in prefile_send(). I have to admit, I don't fully understand it, but single-stepping in the debugger reveals that it loops three times: First, it sends refbuf no. 1 (the headers), then it deallocates the first refbuf and moves refbuf 2 up the chain, then it sends refbuf no. 2. And then it returns -1 because CLIENT_AUTHENTICATED is not set. I'm not sure if this is related to my test case or a bug.

karlheyes commented 8 years ago

Whee!, a few comments here, lets see what we can make of this. I think the main issue is actually dealt with already in post kh3 work, check the master branch to make sure. I think there is still an issue with a case not starting with 0 but I need to verify that.

The prefile case is there mainly for in-memory content eg ,xsl, error responses (eg 404), it's not for the actually reading a file, that instead is done by the throttle/file read routine. prefile is there to handle chained buffers and the client may not be authenticated.

The main oddity with the buffer filling for the stupid cases clients do like iphone doing a range request of 0-1. The other oddity is handling cases like HEAD requests (headers but no content) and avoiding triggering auth for these small requests from players.

karl.

onitake commented 8 years ago

I see. So, the proper fix will simply be to not allocate any chained buffer at all if an actual file is served? That certainly sounds much simpler.

We'll check out the HEAD version then, and hopefully the problem is already fixed there.

onitake commented 8 years ago

Looks like this fixes our problem already. Nice.

When do you plan to release the next icecast version? In the meantime, we'll just work with the current HEAD instead.

Thanks!

karlheyes commented 8 years ago

I really do want to get kh4 out, Each time I have had time to do the release, someone has come up with some issue to look into. I think there is just the silly issue I mentioned previously which looks to be down to when something is initialised. Hopefully this week....

karl.

onitake commented 8 years ago

No problem, take your time. Better iron out those pesky bugs first. :+1: