Dev image does not send proper url for webhook callbacks #1739

naanlizard · 2024-11-14T15:34:57Z

Describe the bug
On the latest dev image (airensoft/ovenmediaengine@sha256:773acc79230548f8d942b2321f94d10e0846135128a5b0dc35eab60ff0fa528e), admission webhooks do not work because the URL sent does not include the streamkey

To Reproduce
Steps to reproduce the behavior:
Server.xml

(Note, in Server.xml, the Decodes section is either commented out (for 0.17.1) or uncommented (for dev)

Dockerfile

Maybe relevant, if you put the streamkey in the URL (e.g. in OBS, your rtmp url is set to rtmp://ingest.hostname.tld:1935/live/streamkey) both versions work.

Expected behavior
URL sent should include the streamkey, like on 0.17.1.

Logs
Relevant log lines from our backend API, Parameters being the values from the request OME sends

0.17.1
Parameters: {"client"=>{"address"=>"x", "port"=>58323, "real_ip"=>"x"}, "request"=>{"direction"=>"incoming", "protocol"=>"RTMP", "status"=>"opening", "time"=>"2024-11-14T15:23:54.039+00:00", "url"=>"rtmp://ingest.hostname.tld:1935/live/streamkey"}, "stream"=>{}}

dev
Parameters: {"client"=>{"address"=>"x", "port"=>58619, "real_ip"=>"x"}, "request"=>{"direction"=>"incoming", "protocol"=>"RTMP", "status"=>"opening", "time"=>"2024-11-14T15:28:11.658+00:00", "url"=>"rtmp://ingest.hostname.tld:1935/live"}, "stream"=>{}}

Additional info:

~~Interestingly, it seems like the closing webhook does send the streamkey in the URL~~

Request params: {"direction"=>"incoming", "new_url"=>"rtmp://ingest.hostname.tld:1935/live/streamkey", "protocol"=>"RTMP", "status"=>"closing", "time"=>"2024-11-14T15:27:15.762+00:00", "url"=>"rtmp://apollo-rtmp://ingest.hostname.tld:1935/live/streamkey"}

Double checked the above, it seems like it does NOT send the streamkey in the closing call, or at least not always. Strange

The text was updated successfully, but these errors were encountered:

…ing the URL (#1739)

dimiden · 2024-11-16T11:07:36Z

@naanlizard
Thank you for the report!
Upon reviewing the issue you mentioned, I found that a recent improvement in the URL parsing process caused the problem.
I have just fixed this issue and committed it as 9f92959.

naanlizard · 2024-11-16T17:23:28Z

The proper URL is now sent, but there's a new problem, which I'm not sure how to describe

When I start, the stream immediately stops, even though the backend returns an authorized

Backend logs (slightly modified for clarity with some lines removed for irrelevant things our backend is doing - fundamentally, it's doing the right thing I think is the point, this is the same code as on our production server)

I, [2024-11-16T17:17:51.137604 #190]  INFO -- : [319dc37e-f979-4892-a80b-2d1ab2ead660] Started POST "/streams/publish" for x at 2024-11-16 17:17:51 +0000
I, [2024-11-16T17:17:51.139766 #190]  INFO -- : [319dc37e-f979-4892-a80b-2d1ab2ead660] Processing by StreamsController#publish as JSON
I, [2024-11-16T17:17:51.139873 #190]  INFO -- : [319dc37e-f979-4892-a80b-2d1ab2ead660]   Parameters: {"client"=>{"address"=>"x", "port"=>56813, "real_ip"=>"x"}, "request"=>{"direction"=>"incoming", "protocol"=>"RTMP", "status"=>"opening", "time"=>"2024-11-16T17:17:51.135+00:00", "url"=>"rtmp://subdomain-ingest.hostname.tld:1935/live/streamkey"}, "stream"=>{}}
I, [2024-11-16T17:17:51.140378 #190]  INFO -- : [319dc37e-f979-4892-a80b-2d1ab2ead660] Starting publish action
I, [2024-11-16T17:17:51.140454 #190]  INFO -- : [319dc37e-f979-4892-a80b-2d1ab2ead660] Request params: {"direction"=>"incoming", "protocol"=>"RTMP", "status"=>"opening", "time"=>"2024-11-16T17:17:51.135+00:00", "url"=>"rtmp://subdomain-ingest.hostname.tld:1935/live/streamkey"}
I, [2024-11-16T17:17:51.140556 #190]  INFO -- : [319dc37e-f979-4892-a80b-2d1ab2ead660] Extracted token: streamkey
D, [2024-11-16T17:17:51.142152 #190] DEBUG -- : [319dc37e-f979-4892-a80b-2d1ab2ead660]   Stream Load (0.7ms)  SELECT `streams`.* FROM `streams` WHERE `streams`.`id` = 1 LIMIT 1
I, [2024-11-16T17:17:51.143132 #190]  INFO -- : [319dc37e-f979-4892-a80b-2d1ab2ead660] Fetched stream: #<Stream id: 1, title: "Stream", description: nil, slug: "username", token: "streamkey", live_since: nil, adult: true, in_multi: false, parent_stream_id: nil, user_id: 1, created_at: "2024-11-16 02:00:57.721218000 +0000", updated_at: "2024-11-16 17:16:35.570224000 +0000", rendered_description: "<p></p>", last_seen: "2024-11-16 17:16:35.568285000 +0000", offline_image: nil, banner: nil, banner_link: nil, viewers_count: 0, preview: nil>
D, [2024-11-16T17:17:51.144171 #190] DEBUG -- : [319dc37e-f979-4892-a80b-2d1ab2ead660]   User Load (0.3ms)  SELECT `users`.* FROM `users` WHERE `users`.`id` = 1 LIMIT 1
I, [2024-11-16T17:17:51.144478 #190]  INFO -- : [319dc37e-f979-4892-a80b-2d1ab2ead660] Authorization succeeded. Setting stream to live.

I, [2024-11-16T17:17:51.178706 #91]  INFO -- : [23f53614-0876-4402-a453-871142cd4b3e] Started POST "/streams/publish" for x at 2024-11-16 17:17:51 +0000
I, [2024-11-16T17:17:51.180101 #91]  INFO -- : [23f53614-0876-4402-a453-871142cd4b3e] Processing by StreamsController#publish as JSON
I, [2024-11-16T17:17:51.180216 #91]  INFO -- : [23f53614-0876-4402-a453-871142cd4b3e]   Parameters: {"client"=>{"address"=>"x", "port"=>56813, "real_ip"=>"x"}, "request"=>{"direction"=>"incoming", "new_url"=>"://", "protocol"=>"RTMP", "status"=>"closing", "time"=>"2024-11-16T17:17:51.177+00:00", "url"=>"rtmp://subdomain-ingest.hostname.tld:1935/live/streamkey"}, "stream"=>{}}
I, [2024-11-16T17:17:51.180688 #91]  INFO -- : [23f53614-0876-4402-a453-871142cd4b3e] Starting publish action
I, [2024-11-16T17:17:51.180754 #91]  INFO -- : [23f53614-0876-4402-a453-871142cd4b3e] Request params: {"direction"=>"incoming", "new_url"=>"://", "protocol"=>"RTMP", "status"=>"closing", "time"=>"2024-11-16T17:17:51.177+00:00", "url"=>"rtmp://subdomain-ingest.hostname.tld:1935/live/streamkey"}
I, [2024-11-16T17:17:51.180807 #91]  INFO -- : [23f53614-0876-4402-a453-871142cd4b3e] Extracted token: streamkey
D, [2024-11-16T17:17:51.181696 #91] DEBUG -- : [23f53614-0876-4402-a453-871142cd4b3e]   Stream Load (0.5ms)  SELECT `streams`.* FROM `streams` WHERE `streams`.`id` = 1 LIMIT 1
I, [2024-11-16T17:17:51.182434 #91]  INFO -- : [23f53614-0876-4402-a453-871142cd4b3e] Fetched stream: #<Stream id: 1, title: "Stream", description: nil, slug: "garrett", token: "streamkey", live_since: "2024-11-16 17:17:51.145101000 +0000", adult: true, in_multi: false, parent_stream_id: nil, user_id: 1, created_at: "2024-11-16 02:00:57.721218000 +0000", updated_at: "2024-11-16 17:17:51.146750000 +0000", rendered_description: "<p></p>", last_seen: "2024-11-16 17:16:35.568285000 +0000", offline_image: nil, banner: nil, banner_link: nil, viewers_count: 0, preview: nil>
D, [2024-11-16T17:17:51.183619 #91] DEBUG -- : [23f53614-0876-4402-a453-871142cd4b3e]   User Load (0.4ms)  SELECT `users`.* FROM `users` WHERE `users`.`id` = 1 LIMIT 1
I, [2024-11-16T17:17:51.183960 #91]  INFO -- : [23f53614-0876-4402-a453-871142cd4b3e] Stream is closing. Stream ID: 1

omebug111624.log-clean.txt

Dockerfile is the same as above but with

FROM airensoft/ovenmediaengine@sha256:0510bb6178a96083139a66b601c8aceeba5bb787f2e5694c8abfbcbb0010edef

dimiden · 2024-11-18T01:42:35Z

@naanlizard
Oh no... The logs show there’s an issue. I thought the URL parser was modified to be compatible with the previous code, but it seems there are unexpected side effects.
I will carefully check if there are any other issues besides the one you mentioned and get back to you!

dimiden · 2024-11-18T13:50:57Z

@naanlizard
The issue occurred because a different value was returned for invalid URL, such as empty URL, compared to the previous implementation.
This issue has been fixed in 4c4ca98, and it has been confirmed that it works correctly even if empty URL is responsed from admission webhook server.

Thank you for the testing!

naanlizard · 2024-11-18T14:31:18Z

Thanks! I'll test it asap Can you clarify what the admission server should return for that url value? Are we doing something unusual?

…

On Mon, Nov 18, 2024 at 08:51 Hyunjun Jang ***@***.***> wrote: @naanlizard <https://github.com/naanlizard> The issue occurred because a different value was returned for invalid URL, such as empty URL, compared to the previous implementation. This issue has been fixed in 4c4ca98 <4c4ca98>, and it has been confirmed that it works correctly even if empty URL is responsed from admission webhook server. Thank you for the testing! — Reply to this email directly, view it on GitHub <#1739 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABV4BWMUZ6L4BM65KRVUFQL2BHWFTAVCNFSM6AAAAABRZFZYSSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBTGEYDQMRZGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

dimiden · 2024-11-18T14:42:55Z

According to my analysis, it seems that you are assigning "" to new_url, which essentially behaves as if an invalid URL is being provided.
If there is no URL to redirect to, it would be better to either omit new_url entirely or set it to null.

naanlizard · 2024-11-18T14:47:46Z

Hmm, I think we set a new_url, but I will double check and get back to you

…

On Mon, Nov 18, 2024 at 09:43 Hyunjun Jang ***@***.***> wrote: According to my analysis, it seems that you are assigning "" to new_url, which essentially behaves as if an invalid URL is being provided. If there is no URL to redirect to, it would be better to either omit new_url entirely or set it to null. — Reply to this email directly, view it on GitHub <#1739 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABV4BWIWD6NQ2GYA6NU4RVD2BH4ILAVCNFSM6AAAAABRZFZYSSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBTGI2TEOJVGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

dimiden · 2024-11-18T14:54:04Z

Yes. If new_url is not empty, the issue might be with the regular expression used for URL parsing. It would be helpful if you could provide the URL being passed as new_url by the server.
(Since the URL pattern is important, you may replace sensitive information with arbitrary characters.)

For reference, I tested the parser with various URL combinations and encountered no issues, so I suspect there’s a high probability that a empty string was responsed.

naanlizard · 2024-11-18T14:55:24Z

It could be a bug in our backend code, I'll verify the url first 👍

…

On Mon, Nov 18, 2024 at 09:54 Hyunjun Jang ***@***.***> wrote: Yes. If new_url is not empty, the issue might be with the regular expression used for URL parsing. It would be helpful if you could provide the URL being passed as new_url by the server. (Since the URL pattern is important, you may replace sensitive information with arbitrary characters.) For reference, I tested the parser with various URL combinations and encountered no issues, so I suspect there’s a high probability that a empty string was responsed. — Reply to this email directly, view it on GitHub <#1739 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABV4BWIVIAIV4OVXFOCJFH32BH5SHAVCNFSM6AAAAABRZFZYSSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBTGI4DMNJUGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

hernanrz · 2024-11-20T17:58:49Z

Looks like the latest dev docker image fixed this, we were responding to OME admission webhook with the following response, which wasn't working on image hash sha256:0510bb6178a96083139a66b601c8aceeba5bb787f2e5694c8abfbcbb0010edef

{"allowed":true,"new_url":"live/1"}

After further testing on that image I found that if you set the url to an absolute URL, it worked again

After pulling the latest dev docker image sha256:103e6e0efafe507c6e6f3065d457a1fd2db11ce87a2a74539db8da963716a70a it works again with the relative URL format

naanlizard · 2024-11-20T18:30:59Z

See hernan's comment above - what is the intended use case for new_url?

For our purpose, we have users connect to rtmp://hostname.tld:1935/live and put userid?streamkey in their streamkey slot in OBS. So the URL OME sees is in sum, rtmp://hostname.tld:1935/live/userid?streamkey

Should we be returning the /live/userid value, so that the playback url is correct (e.g. https://hostname.tld/live/1/llhls.m3u8) or is that unnecessary?

If so, should new_url be the full URL (e.g. rtmp://hostname.tld:1935/live/userid) or just the relative path (e.g. /live/userid)

I'll make a pull request for documentation improvements once I understand

choigilhoon · 2024-11-21T08:07:23Z

@naanlizard

First, the new_url should be set in the form of a full URL (scheme://host[:port]/app/stream/file?query=value&query2=value2). (Please refer to this)
Then, depending on the form of the playback URL, the value of new_url should change.

Below are two use cases for setting new_url:
[Use case 1]
If you want to hide the streamkey and expose only the userid in the playback URL, such as https://hostname.tld/live/{userid}/llhls.m3u8, set new_url as follows:
"rtmp://hostname.tld:1935/live/{userid}"

[Use case 2]
If you want to hide the userid and expose only the streamkey in the playback URL, such as https://hostname.tld/live/{streamkey}/llhls.m3u8, set new_url as follows:
"rtmp://hostname.tld:1935/live/{streamkey}"

And, if new_url is not a full URL, this value will be ignored, and the value set in OBS (rtmp://hostname.tld:1935/live/userid?streamkey) will be used.

This explains why, even though you entered a relative path in new_url, it seems to work properly because the stream name is automatically set to {userid}, as in [Use case 1].

naanlizard · 2024-11-28T18:11:09Z

To be clear @choigilhoon -

In a vanilla OME install/config, sending an OBS url and streamkey like

rtmp://hostname.tld:1935/appname

and

abc?xyz123 (or any random strings separated by a ?)

Will lead to OME serving that stream as

https://hostname.tld/appname/abc/llhls.m3u8

Correct?

choigilhoon · 2024-12-03T12:28:47Z

@naanlizard

That's correct.

In the situation you mentioned, you should respond to the OME admission webhook with one of the cases below.

Case 1

{
  "allowed": true
}

Case 2

{
  "allowed": true,
  "new_url": "rtmp://hostname.tld:1935/appname/abc"
}

Please note that the query-string part of the OBS streamkey (?xyz123) is ignored when generating the playback URL.

…ing the URL (AirenSoft#1739)

dimiden self-assigned this Nov 16, 2024

dimiden added the in progress Being actively worked on but may take some time to complete label Nov 16, 2024

dimiden added a commit that referenced this issue Nov 16, 2024

Fixed issue where the stream name was not applied to _path after pars…

9f92959

…ing the URL (#1739)

dimiden added bug Confirmed as bug patched Patch applied and removed in progress Being actively worked on but may take some time to complete labels Nov 16, 2024

dimiden added in progress Being actively worked on but may take some time to complete and removed patched Patch applied labels Nov 18, 2024

dimiden added patched Patch applied and removed in progress Being actively worked on but may take some time to complete labels Nov 18, 2024

negtak pushed a commit to negtak/OvenMediaEngine that referenced this issue Dec 11, 2024

Fixed issue where the stream name was not applied to _path after pars…

26647a8

…ing the URL (AirenSoft#1739)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dev image does not send proper url for webhook callbacks #1739

Dev image does not send proper url for webhook callbacks #1739

naanlizard commented Nov 14, 2024 •

edited

Loading

dimiden commented Nov 16, 2024

naanlizard commented Nov 16, 2024 •

edited

Loading

dimiden commented Nov 18, 2024

dimiden commented Nov 18, 2024

naanlizard commented Nov 18, 2024 via email

dimiden commented Nov 18, 2024

naanlizard commented Nov 18, 2024 via email

dimiden commented Nov 18, 2024

naanlizard commented Nov 18, 2024 via email

hernanrz commented Nov 20, 2024

naanlizard commented Nov 20, 2024

choigilhoon commented Nov 21, 2024

naanlizard commented Nov 28, 2024

choigilhoon commented Dec 3, 2024

Dev image does not send proper url for webhook callbacks #1739

Dev image does not send proper url for webhook callbacks #1739

Comments

naanlizard commented Nov 14, 2024 • edited Loading

dimiden commented Nov 16, 2024

naanlizard commented Nov 16, 2024 • edited Loading

dimiden commented Nov 18, 2024

dimiden commented Nov 18, 2024

naanlizard commented Nov 18, 2024 via email

dimiden commented Nov 18, 2024

naanlizard commented Nov 18, 2024 via email

dimiden commented Nov 18, 2024

naanlizard commented Nov 18, 2024 via email

hernanrz commented Nov 20, 2024

naanlizard commented Nov 20, 2024

choigilhoon commented Nov 21, 2024

naanlizard commented Nov 28, 2024

choigilhoon commented Dec 3, 2024

naanlizard commented Nov 14, 2024 •

edited

Loading

naanlizard commented Nov 16, 2024 •

edited

Loading