Add webradio stream title with icecast/shoutcast #987
Conversation
|
Thanks! I'll check this out as soon as I have time. One quick comment though: icecast-metadata-js is a 3rd party library and it should be available as an NPM package. So there should be no need to copy it to our version control. It should be sufficient to run On the other hand, if a 3rd party library couldn't be obtained as a node module, then it should reside under |
|
Thank you for review, icecast-metadata-js can be added from npm this is the repo: https://github.com/eshaz/icecast-metadata-js/tree/master/src/icecast-metadata-js#readme |
| @@ -76,6 +76,7 @@ function adjustCsp(IAppContainer $container) { | |||
|
|
|||
| foreach ($radioSources as $source) { | |||
| $policy->addAllowedMediaDomain($source); | |||
| $policy->addAllowedConnectDomain($source); | |||
There was a problem hiding this comment.
This is somewhat problematic because by default, this now makes a rather large hole to the CSP. The default allowed radio sources is *:* so after this, any javascript running within ownCloud/Nextcloud is allowed to connect which ever URL it likes. This basically undermines the whole point of the Content Security Policy. Also, this is the reason, why I require each cloud administrator to whitelist the HLS radio sources they like to allow: Playing the HLS streams requires this connect-src CSP rule, and I don't want to automatically weaken the security of each and every cloud instance running this app.
Maybe the solution might be to relay all this metadata loading via our own server? So the Music app PHP backend would have a REST API to get the radio metadata and its implementation would make the calls to the icecast/shoutcast servers. I believe this would help also with the cases where the icecast/shoutcast server doesn't define the needed CORS headers.
There was a problem hiding this comment.
I agree that with php it will be easier and more secure, I was thinking that using the server to load information is more bad, if you agree I will try new method with php in the server.
There was a problem hiding this comment.
Please do. That shouldn't generate too much server load as the metadata is loaded with rather limited rate.
PS. I'll be busy with other things for the next week, so I probably can't give any more feedback during the next ~7 days. But I will review any updates as soon possible.
|
|
||
| var methodHandler = undefined; | ||
|
|
||
| switch (method) { |
There was a problem hiding this comment.
Apparently the code is now always using the "STREAM" method? Is there any reason to have these others here? In which case would they be useful?
There was a problem hiding this comment.
Yes, I was preparing add support to for shoutcast stream title using url parsing, most of radios today use icecast metadata so is not important.
|
The inspection completed: No new issues |
|
superseded by #992 |
Add support for webradio stream title