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

fix: add https for notify url #1683

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

lbaron-ut
Copy link

When using the ngx_rtmp_notify_commands for different events such as on_connect or on_publish, we want to notify an authentication service.

We use these directives in the nginx.conf and provide the URL of the authentication service.

However it doesn't work when the authentication service is using https.

This is because the ngx_rtmp_notify_parse_url in ngx_rtmp_notify_module.c method does NOT handle https.

    if (ngx_strncasecmp(url->data, (u_char *)"http://", 7) == 0)
    {
        add = 7;
    }

I propose a very simple fix:

    if (ngx_strncasecmp(url->data, (u_char *)"http://", 7) == 0)
    {
        add = 7;
        port = 80
    }
    else if (ngx_strncasecmp(url->data, (u_char *)"https://", 8) == 0)
    {
        add = 8;
        port = 443;
    }

This fix is very similar to these:
https://mailman.nginx.org/pipermail/nginx-devel/2013-August/004065.html
https://github.com/kaltura/nginx-aws-auth-module/blob/dbbac974f0699328a63f497cd911a4f991faed9b/ngx_http_aws_auth_module.c#L992-L1009

@spjoes
Copy link

spjoes commented May 30, 2023

I know this is quite old but everytime I try using it I get, invalid port in url "https://MY_DOMAIN_NAME.com/api/endpoint_here" in /usr/local/nginx/nginx.conf:141

here is my rtmp block aswell if that helps:

rtmp {
        server {
                listen 1935;
                chunk_size 4096;
                allow publish 127.0.0.1;
		allow publish all;
                deny publish all;

                application live {
			on_publish https://MY_DOMAIN_NAME.com/api/endpoint_here;
                        
			live on;
                        record off;
			hls on;
                        hls_path /usr/local/nginx/html/stream/hls;
                        hls_fragment 3;
                        hls_playlist_length 60;

                        dash on;
                        dash_path /usr/local/nginx/html/stream/dash;
                }
        }
}

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

Successfully merging this pull request may close these issues.

3 participants