ossrs / srs

SRS is a simple, high-efficiency, real-time video server supporting RTMP, WebRTC, HLS, HTTP-FLV, SRT, MPEG-DASH, and GB28181.
https://ossrs.io
MIT License
24.72k stars 5.28k forks source link

Bug: Config http_api and rtc_server.tcp listen on same port. #4026

Open suzp1984 opened 2 months ago

suzp1984 commented 2 months ago

Describe the bug I notice http_api & http_server can listen on same endpoint, and rtc_server.tcp & http_server can also listen on same endpoint. https://github.com/ossrs/srs/blob/427104f1dab86f5afc7d7b49b02ed27d03ef9346/trunk/src/app/srs_app_server.cpp#L351-L352

But there are a lot of SrsTcpListeners there, can they shared with same endpoints just like reuse_api_over_server_ & reuse_rtc_over_server_?

e.g. can we reuse rtc over http_api?

http_api {
    enabled         on;
    listen          1985;
}
stats {
    network         0;
}
rtc_server {
    enabled on;
    tcp {
        enabled on;
        listen 1985;
    }
    protocol tcp;
    # @see https://ossrs.net/lts/zh-cn/docs/v4/doc/webrtc#config-candidate
    candidate $CANDIDATE;
}

change conf/rtc.tcp.only.conf, let http_api.listen and rtc_server.tcp.listen with same port. then boot the srs: ./objs/srs -c conf/rtc.tcp.only.conf

HTTP-API listen at tcp://0.0.0.0:1985, fd=10
WebRTC listen at tcp://0.0.0.0:1985, fd=12

I don't think the http_api and webRTC over TCP can still works well in this case.

Version ALL SRS version

To Reproduce Steps to reproduce the behavior: not just rtc_server.tcp.listen conflict with http_api.listen, rtmp can also be configed conflict with http_server.listen. e.g.

  1. edit to 'trunk/conf/http.hls.conf', rewrite http_server.listen to 1935;
  2. boot srs: ./objs/srs -c conf/http.hls.conf;
  3. visit http://127.0.0.1:1935, the RTMP listener will hijack the connection. the web browser can't get response.

Expected behavior Disallow tcp listen have same value, except the already supported reuse_api_over_server_ and reuse_rtc_over_server_. Or support more tcp listener reuse cases, e.g. rtc & http_api, and disallow others conflicts.

Solutions support reuse TcpListeners, but there are a lot of them: https://github.com/ossrs/srs/blob/427104f1dab86f5afc7d7b49b02ed27d03ef9346/trunk/src/app/srs_app_server.cpp#L332-L341 We can do more things, by do support more reuse listen cases, and do more SrsConfig::check_config, then send panic earlier.

winlinvip commented 2 months ago

APIs and WebRTC are capable of functioning together, and the type of protocol can be determined based on the content of the body. If they do not work well together, it is actually due to issues with the software implementation rather than the protocols being unable to recognize each other.

Since the Listener is a mechanism that is continuously patched, it is not considered reasonable. A better approach would be to completely redesign the mechanism of the Listener.

It is important to note that the SRT's listener requires special handling. For reference, see the issue at https://github.com/ossrs/srs/issues/3251#issuecomment-2046209863. This issue necessitates improvements to the current structure.

TRANS_BY_GPT4

suzp1984 commented 2 months ago

Let APIs and WebRTC works on same endpoints would be easy, just refer the reuse_rtc_over_server_ code will add this feature, and really make sense, if reuse_api_over_server_ & reuse_rtc_over_server_ can be both true, then why not another reuse_rtc_over_api_? This is a small patch, can introduce a tiny but controllable complexity.

I like the SrsTcpListener's design, it's really light weight, lower-level abstraction and flexible, maybe I didn't know more better one.

One more thing to consider, the listen config, I notice the inconsistence here. https://github.com/ossrs/srs/blob/427104f1dab86f5afc7d7b49b02ed27d03ef9346/trunk/src/app/srs_app_listener.cpp#L256-L261 https://github.com/ossrs/srs/blob/427104f1dab86f5afc7d7b49b02ed27d03ef9346/trunk/src/kernel/srs_kernel_utility.cpp#L239-L257

https://github.com/ossrs/srs/blob/427104f1dab86f5afc7d7b49b02ed27d03ef9346/trunk/src/app/srs_app_config.hpp#L415-L418 https://github.com/ossrs/srs/blob/427104f1dab86f5afc7d7b49b02ed27d03ef9346/trunk/src/app/srs_app_config.hpp#L1028-L1029

    virtual std::string get_http_api_listen();
    virtual std::string get_https_api_listen();
    // Get the http stream listen port.
    virtual std::string get_http_stream_listen();
    virtual std::string get_https_stream_listen();
    virtual std::string get_exporter_listen();

For the RTMP, http_api, http_server, the listen config means endpoint = [ip4/6 | any address] + port;

But for the rest of the listens, which means just port number (or just ipv4 loop back address + port number).

    virtual std::vector<std::string> get_listens();
    // Get the listen port of stream caster.
    virtual int get_stream_caster_listen(SrsConfDirective* conf);
    // Get the sip.listen port configuration.
    virtual int get_stream_caster_sip_listen(SrsConfDirective* conf);
    virtual int get_rtc_server_listen();
    virtual int get_rtc_server_tcp_listen();
    // Get the srt service listen port
    virtual unsigned short get_srt_listen_port();

Here is how gb28181 config its listener, I think it lost its flexibility. https://github.com/ossrs/srs/blob/427104f1dab86f5afc7d7b49b02ed27d03ef9346/trunk/src/app/srs_app_gb28181.cpp#L417-L439

Because srs_any_address_for_listener() will return the ipv4 loopback in priority. I would rather suggest use a string to represent the listen config, at least it have the capacity of ipv6 address and explicit ip addresses (for the machine with multi physical and virtual network interfaces).