svrooij / node-sonos-ts

:speaker: Sonos control library, use this library in your own appliction.
https://sonos-ts.svrooij.io/
MIT License
81 stars 18 forks source link

TrackUri encoding when setting is incorrect #62

Closed svrooij closed 2 years ago

svrooij commented 4 years ago

The code below needs some work, can someone point out how it should be encoded? From src/helpers/xmlhelper.ts

  static EncodeTrackUri(trackUri: string): string {
    if (trackUri.startsWith('http')) return encodeURI(trackUri);
    if (trackUri.startsWith('x-sonos-hta')) return trackUri;

    // Part below needs some work.
    const index = trackUri.indexOf(':') + 1;
    return trackUri.substr(0, index) + this.EncodeXml(trackUri.substr(index)).replace(/:/g, '%3a');
  }
dgmltn commented 3 years ago

From what I can tell, SoCo doesn't encode anything. It only changes "http:" or anything before the first colon to "x-rincon-mp3radio". I'm not sure I agree with that either. The EncodeXml seems reasonable, to ensure invalid xml is never sent. But it might be better to leave the other specific use case encodings (http, x-sonos-hta, ":") up to the caller.

I'm sure these cases ☝️are in here for a specific historical reason (maybe spotify?) but they don't seem universally correct. For instance, the "http" one is just not correct anymore. I verified that Sonos doesn't know what to do with an http uri.

https://github.com/SoCo/SoCo/blob/527876acc234daba10525574f835238de1ad64a1/soco/core.py#L580