karlheyes / icecast-kh

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

Izicast not connecting with KH17 #392

Closed 20com closed 1 year ago

20com commented 1 year ago

Hi. We use Izicast (https://izicast.de/) as client, but it can't connect with KH17 (I've tried too with 17.1, 17.2 and master). If I downgrade to KH15, it works fine. Also with original Icecast, of course. Any idea? Thanks!!

karlheyes commented 1 year ago

what messages do you see in the error log at the time of the connection by the app.

20com commented 1 year ago

Just it keeps "connecting..."

karlheyes commented 1 year ago

change level to 4 and restart icecast, then try to connect with the app, then check the error log. You can post here or email privately. I just need need to see what is happening. If you can send your xml for icecast so I can see what options you have set (you can blank out user/pass/IPs etc)

Daniel-Noethen commented 1 year ago

Hi, I am the dev of iziCast. For some reason kh17 does not send a response (HTTP/1.0 200 OK) . I have attached the log file of kh17 and a .pcap file that shows the communication. Hope it helps. iziCast_kh17_debug_files.zip

karlheyes commented 1 year ago

thanks, I'll check into it

karlheyes commented 1 year ago

hmm, feeding the details here was ok, had the response as intended. The log seems to indicate a connection then a drop of the client after about 5 secs. could you strace the icecast and send that to me? Just wondering if there is some build combination that may be incorrect.

karl

20com commented 1 year ago

Thanks to both. Here is the Icecast log too, just in case. error.log

Daniel-Noethen commented 1 year ago

could you strace the icecast and send that to me?

Sure, result of

strace -o icecast_kh-git_master.strace icecast-kh.git/src/icecast -c icecast_kh-git.xml

is attached.

The log seems to indicate a connection then a drop of the client after about 5 secs

iziCast drops the connection and tries again after 5 seconds if it didn't receive a server response.

Btw. my icecast config is based on icecast_minimal.xml. Only port, password and log names/level changed.

icecast_kh-git_master.strace.txt

karlheyes commented 1 year ago

I should of said a bit more, strace with -f -tt so that the workers are monitored.

karl

Daniel-Noethen commented 1 year ago

Attached strace -o icecast_kh-git_master.strace -f -tt icecast-kh.git/src/icecast -c icecast_kh-git.xml

icecast_kh-git_master.strace-2.txt

karlheyes commented 1 year ago

a bit of a puzzler, I see one read of 25 bytes (the SOURCE) line, then a second read of 234 bytes (starting with AUTHORI...) then reading attempts but not getting anything.

1397 16:37:58.802384 recvfrom(14, "SOURCE /stream HTTP/1.0\r\n", 4095, 0, NULL, NULL) = 25 1397 16:37:58.843010 recvfrom(14, "Authorization: Basic c291cmNlOml"..., 4070, 0, NULL, NULL) = 234 1397 16:37:58.882696 recvfrom(14, 0x7f7760000e23, 3836, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)

While you could add the -s 300 to strace to verify. It would indicate a storage of headers as the one listed in the pcap show 269 bytes (25 +244). Which would indicate the blank line is not getting seen, therefore the headers are not being processed and everything after that.

Just for comparison I simulated the same action, with a different auth header but same length.

1332830 16:00:31.719328 recvfrom(18, "SOURCE /stream HTTP/1.0\r\n", 4095, 0, NULL, NULL) = 25 1332830 16:00:31.760314 recvfrom(18, "Authorization: Basic c291cmNlOmhhY2ttZQ==\r\nHost: 159.69.109.111:7011\r\nUser-Agent: iziCast v1.6.4 (Build 1)\r\nContent-Type: audio/mpeg\r\nice-name: icecast-kh15\r\nice-public: 0\r\nice-audio-info: ice-bitrate=128;ice-channels=2;ice-samplerate=44100\r\n\r\n", 4070, 0, NULL, NULL) = 244

as you can see, the blank line is present in my test run. You could try to verify with strace -s 300 -f -tt so see what you get but at only 234 bytes then it is 10 short of what I would expect. However the pcap shows it present.

The only thing that springs to mind is the CORK setting having some indirect effect which we could disable, it's less of an issue these days. If you want to disable cork then edit net/sock.c :313 to say #if 0

Daniel-Noethen commented 1 year ago

Hello Karl,

unfortunately, disabling CORK didn't help.

However I was able to track it down to commit 46a3d522.

If I replace

ptr = memmem (buf, ret, "\r\n\r\n", 4);

with

ptr = strstr (buf, "\r\n\r\n");

in the master it is working again!

Seems like the "ret" value is too small, so memmem() stops searching before it reaches "\r\n\r\n". However strstr() finds it because it searches until it reaches the terminating '\0'.

Daniel-Noethen commented 1 year ago

I have put some printf statements below the memmem() line, maybe it helps:

ret: 25
ptr: (null)
buf: SOURCE /stream HTTP/1.0

ret: 238
ptr: (null)
buf: SOURCE /stream HTTP/1.0
Authorization: Basic c291cmNlOnRlbXBwYXNz
Host: 159.69.109.111:7011
User-Agent: iziCast v1.6.4 (Build 1)
Content-Type: audio/mpeg
ice-name: kh-git
ice-public: 0
ice-audio-info: ice-bitrate=128;ice-channels=2;ice-samplerate=44100
karlheyes commented 1 year ago

I was wondering about the memmem change. Can you check if another change works instead seeing that you have a case showing this.

ptr = memmem (buf, refbuf->len, "\r\n\r\n", 4);

you should only need the 1 changed as other EOL are not common.

Daniel-Noethen commented 1 year ago

That works!

karlheyes commented 1 year ago

ok great, I'll tie that up shortly, just need to verify another code path for this

karlheyes commented 1 year ago

ok, committed to master, you should be good to go.

karl

Daniel-Noethen commented 1 year ago

Yes, it works :) Thanks!

20com commented 1 year ago

Thank you both for your fast and good job :)

karlheyes commented 1 year ago

thanks for chasing this up.