Skip to content
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

[1.x] RTSP SETUP does not handle errors #3015

Closed
AWilco opened this issue Jul 1, 2022 · 2 comments
Closed

[1.x] RTSP SETUP does not handle errors #3015

AWilco opened this issue Jul 1, 2022 · 2 comments
Labels
multistream Related to Janus 1.x

Comments

@AWilco
Copy link

AWilco commented Jul 1, 2022

What version of Janus is this happening on?
52f86d5

When connecting to an RTSP source that doesn't respond with a 200 OK request, Janus assumes it did get a 200 OK and continues.

https://github.com/meetecho/janus-gateway/blob/db3300d9263b353b247cb3d9316cb1e87e83dc1a/src/plugins/janus_streaming.c#L7558-7570

My understanding is that this is because the code is not retrieved from the most recent request, so is still using the value from the original DESCRIBE request. The fix would be to get the code after checking that the request was sent ok (or assume the request was OK given we just succeeded in our DESCRIBE request).

      res = curl_easy_perform(curl);
		if(res != CURLE_OK) {
			JANUS_LOG(LOG_ERR, "Couldn't send SETUP request: %s\n", curl_easy_strerror(res));
			g_strfreev(parts);
			curl_easy_cleanup(curl);
			g_free(curldata->buffer);
			g_free(curldata);
			if(video_fds.fd != -1) close(video_fds.fd);
			if(video_fds.rtcp_fd != -1) close(video_fds.rtcp_fd);
			if(audio_fds.fd != -1) close(audio_fds.fd);
			if(audio_fds.rtcp_fd != -1) close(audio_fds.rtcp_fd);
			return -5;
		}
                res = curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &code);
                if(code != 200) {
			JANUS_LOG(LOG_ERR, "Couldn't get SETUP code: %ld\n", code);
			g_strfreev(parts);
			curl_easy_cleanup(curl);
			g_free(curldata->buffer);
			g_free(curldata);
			if(video_fds.fd != -1) close(video_fds.fd);
			if(video_fds.rtcp_fd != -1) close(video_fds.rtcp_fd);
			if(audio_fds.fd != -1) close(audio_fds.fd);
			if(audio_fds.rtcp_fd != -1) close(audio_fds.rtcp_fd);
			return -5;
		}

Similarly this would need to be added at line 7745.
I can create a PR for this if requested.

@AWilco AWilco added the multistream Related to Janus 1.x label Jul 1, 2022
@lminiero
Copy link
Member

lminiero commented Jul 1, 2022

Makes sense, we're already using curl_easy_getinfo to check the response to a DESCRIBE to I guess we should indeed use it for other requests too (and not just SETUP). No need for a PR as you highlighted the problem clearly, I'll prepare a commit for master shortly (even though I guess we'll have to make the same fix in 0.x as well).

@lminiero
Copy link
Member

lminiero commented Jul 1, 2022

Should be fixed now, thanks for spotting this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multistream Related to Janus 1.x
Projects
None yet
Development

No branches or pull requests

2 participants