devgianlu / go-librespot

Yet another open source Spotify client, written in Go.
GNU General Public License v3.0
75 stars 15 forks source link

Cross-Fade & Cross-Origin #27

Closed tylkie closed 5 months ago

tylkie commented 6 months ago

Great job... much easier-to-understand and more stable framework than the java based one. Occam would be delighted.

The only thing I am missing is the cross-fade feature.

Furthermore I like the Player API very much, especially the events, which makes external player state control much easier than using the official Spotify API. In order to be able to access the API from a different domain, cross-origin acceptance must be supplied to the websocket. I added it manually to the code and recompiled. For extreme convenience, it could be read from the configuration file.

devgianlu commented 5 months ago

I have added support for cross origins:

server:
  enabled: true
  port: 3678
  allow_origin: '*'

Let me know if you have an issues. Can you open a separate issue for cross fading?

thilojaeggi commented 2 months ago

Does this not work anymore?

devgianlu commented 2 months ago

Does this not work anymore?

@thilojaeggi should be working

thilojaeggi commented 2 months ago

Hmm, I've added * in the config but "http://localhost:5001" for example gets disallowed.

devgianlu commented 2 months ago

Can you send the headers that you receive?

tylkie commented 2 months ago

Had the same problem with your solution. Cannot say why it does not work. I solved the problem by re-providing the origin patterns when accepting a websocket connection on the events endpoint rather then setting the header before starting to serve.

c, err := websocket.Accept(w, r, &websocket.AcceptOptions{ OriginPatterns: []string{"*"}, })

Hope it helps.

devgianlu commented 2 months ago

Makes sense, I've added this.

tylkie commented 1 month ago

I have been fiddling around with the api server and have come to several conclusions worth to share here...

1.) When using the simple http endpoints, the allowed origin must be identical to the one of the requesting client. If a websocket is accepted, it requires a FQDN without the protocol from which the request originates. For example... if I make requests from https://test.com, than the corresponding code executions within the api server should be...

w.Header().Set("Access-Control-Allow-Origin", "https://test.com") ... for simple https requests. c, err := websocket.Accept(w, r, &websocket.AcceptOptions{ OriginPatterns: []string{"test.com", }) ... for websockets.

If requests are made from http://test.com, you have to use same protocol for http requests...

w.Header().Set("Access-Control-Allow-Origin", "http://test.com") ... for simple http requests. c, err := websocket.Accept(w, r, &websocket.AcceptOptions{ OriginPatterns: []string{test.com}, }) ... for websockets.

If you pass the protocol to the websocket accept function as with the simple http request, websockets won't be accepted. If you forget the protocol with simple http requests, those won't be accepted.

I solved it by providing the full origin with protocol to the config file (https://test.com), then using that value for simple http requsts as is and removing the "https://" for websocket requests.

2.) Currently only one allowed origin can be provided. If you want to be able to use multiple origins without wildcards, then the config code has to be updated to parse multiple occurances of allow_origin or an array of strings.

3.) Currently, your version does not feature secure websockets (wss). I changed that by changing the serve call as follows.

err := http.ServeTLS(s.listener, s.allowOriginMiddleware(m),s.certFile,s.keyFile)

I added the certificate and key file paths to the config. Config server part looks like this here at the moment.

server: address: "192.168.0.1" port: 8060 enabled: true allow_origin: "https://test.com" cert_file: "full.chain.certificate.pem" key_file: "private.key.pem"

4.) While using the api server, I have run into one critical segmentation fault bug without any exception caught. I believe the source of the error is the handling of websocket connection errors. Currently, when a websocket connection exception occurs, the client reference is removed from the internal client list, but without being closed explicitly. So if an exception is thrown, which does not originate from a client-side disconnect, the reference is removed anyway and without informing the client about it. The client then lingers around with the socket being still open... while the server forgot about it. I will provide more information as I run into it reproductively.

5.) For thilojaeggis problem. If you use the wildcard, you should be able to call "http://localhost:5001/" in any case. So my guess here would be that you put in the config file, where it should be "". It must be a string. Without the response header I cannot tell.

6.) Next time I'll just post diff files... :-D