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 DELETE URL problem #3195

Closed
wants to merge 3 commits into from
Closed

Fix DELETE URL problem #3195

wants to merge 3 commits into from

Conversation

neilyoung
Copy link

This PR addresses a problem with DELETE (missing FQDN, discussed here #3177) and fixes some CORS issues, which prevented this client to operate properly in trickle-ICE mode https://github.com/Eyevinn/whip

This PR addresses a problem with DELETE (missing FQDN, discussed here #3177) and fixes some CORS issues, which prevented this client to operate properly in trickle-ICE mode https://github.com/Eyevinn/whip
@neilyoung neilyoung changed the title Update http_server.go Fix DELETE URL problem Apr 9, 2024
Added NGINX detection in order to provide the proper scheme
An even easier approach to solve this problem after discussion
@aler9
Copy link
Member

aler9 commented Apr 13, 2024

I tested both Eyevinn/whip client (https://web.whip.eyevinn.technology/) and OBS 30.1.2, against MediaMTX v1.6.0 and MediaMTX.

All combination worked without changing any header. Changes in this PR are not needed.

@aler9 aler9 closed this Apr 13, 2024
@neilyoung
Copy link
Author

neilyoung commented Apr 13, 2024

I'm sorry, we must be talking about different versions.

I'm here at commit f5d9fde of your repo, Aril 1st.

This is the response given to OBS 30.1.2 on MacOS for a DELETE with the URL, which is (still) relative and by that not working for OBS (and others):

The location header given in the 201 was:

whip/0ab307c5-870d-4eab-8cba-037a0d97d7a3
DELETE /whip/0ab307c5-870d-4eab-8cba-037a0d97d7a3 HTTP/1.1
Host: ai.votix.com:8889
Accept: */*
User-Agent: Mozilla/5.0 (OBS-Studio/30.1.2; Mac OS X; en-US)

HTTP/1.1 404 Not Found
Access-Control-Allow-Credentials: true
Access-Control-Allow-Origin: *
Content-Type: text/plain
Server: mediamtx
Date: Sat, 13 Apr 2024 11:11:05 GMT
Content-Length: 18

404 page not found

It would work if it would be '/stream/whip/0ab307c5-870d-4eab-8cba-037a0d97d7a3'

What am I missing? You could have changed that meanwhile, that would explain it. I didn't pull for a while...

@neilyoung
Copy link
Author

I tested both Eyevinn/whip client (https://web.whip.eyevinn.technology/)

I had a lengthy chat with Eyevinn last week regarding missing CORS headers, but feel free to ignore. I'm not using their solution.

Eyevinn/whip#133

Copy link
Contributor

This issue is mentioned in release v1.7.0 🚀
Check out the entire changelog by clicking here

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.

None yet

2 participants