Merge pull request #94 from svrooij/bug/album-uri-encoding

fix: Metadata decoding
svrooij · Dec 28, 2020 · db69a25 · db69a25
2 parents 1adf2dc + 22a6142
commit db69a25
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 24 deletions.
diff --git a/.github/workflows/test-and-release.yml b/.github/workflows/test-and-release.yml
@@ -27,12 +27,11 @@ jobs:
       run: npm ci
     - name: Build library
       run: npm run build
+    - name: Run code linting
+      run: npm run lint
     - name: Run tests
-      run: npm run test
-    - name: Send data to Coveralls
-      uses: coverallsapp/github-action@master
-      with:
-        github-token: ${{ secrets.GITHUB_TOKEN }}
+      if: ${{ always() }}
+      run: npm run jest
     - uses: svrooij/secret-gate-action@v1
       id: mygate
       with:
@@ -46,3 +45,9 @@ jobs:
       env:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
+    - name: Send data to Coveralls
+      if: ${{ always() }}
+      uses: coverallsapp/github-action@master
+      with:
+        github-token: ${{ secrets.GITHUB_TOKEN }}
+
diff --git a/package.json b/package.json
@@ -6,9 +6,9 @@
   "types": "lib/index.d.js",
   "scripts": {
     "build": "tsc",
+    "jest": "jest --forceExit",
     "lint-fix": "eslint ./src/*.ts ./src/**/*.ts --fix",
     "lint": "eslint ./src/*.ts ./src/**/*.ts",
-    "jest": "jest",
     "prepack": "npm run build",
     "serve-docs": "docker run --rm --volume=\"$PWD/docs/vendor/bundle:/usr/local/bundle\" --volume=\"$PWD/docs:/srv/jekyll\" -p 4000:4000 -p 35729:35729 -it jekyll/jekyll jekyll serve --livereload",
     "test": "npm run lint && jest --forceExit"

diff --git a/src/helpers/metadata-helper.ts b/src/helpers/metadata-helper.ts
@@ -21,10 +21,10 @@ export default class MetadataHelper {
     const parsedItem = didl as {[key: string]: any };
     const didlItem = (parsedItem['DIDL-Lite'] && parsedItem['DIDL-Lite'].item) ? parsedItem['DIDL-Lite'].item : parsedItem;
     const track: Track = {
-      Album: XmlHelper.EncodeXmlUndefined(didlItem['upnp:album']),
-      Artist: XmlHelper.EncodeXmlUndefined(didlItem['dc:creator']),
+      Album: XmlHelper.DecodeHtml(didlItem['upnp:album']),
+      Artist: XmlHelper.DecodeHtml(didlItem['dc:creator']),
       AlbumArtUri: undefined,
-      Title: XmlHelper.EncodeXmlUndefined(didlItem['dc:title']),
+      Title: XmlHelper.DecodeHtml(didlItem['dc:title']),
       UpnpClass: didlItem['upnp:class'],
       Duration: undefined,
       ItemId: didlItem._id,
@@ -35,19 +35,21 @@ export default class MetadataHelper {
     if (didlItem['r:streamContent'] && typeof didlItem['r:streamContent'] === 'string' && track.Artist === undefined) {
       const streamContent = (didlItem['r:streamContent'] as string).split('-');
       if (streamContent.length === 2) {
-        track.Artist = XmlHelper.EncodeXmlUndefined(streamContent[0].trim());
-        track.Title = XmlHelper.EncodeXmlUndefined(streamContent[1].trim());
+        track.Artist = XmlHelper.DecodeHtml(streamContent[0].trim());
+        track.Title = XmlHelper.DecodeHtml(streamContent[1].trim());
       } else {
-        track.Artist = XmlHelper.EncodeXmlUndefined(streamContent[0].trim());
+        track.Artist = XmlHelper.DecodeHtml(streamContent[0].trim());
         if (didlItem['r:radioShowMd'] && typeof didlItem['r:radioShowMd'] === 'string') {
           const radioShowMd = (didlItem['r:radioShowMd'] as string).split(',');
-          track.Title = XmlHelper.EncodeXmlUndefined(radioShowMd[0].trim());
+          track.Title = XmlHelper.DecodeHtml(radioShowMd[0].trim());
         }
       }
     }
     if (didlItem['upnp:albumArtURI']) {
       const uri = Array.isArray(didlItem['upnp:albumArtURI']) ? didlItem['upnp:albumArtURI'][0] : didlItem['upnp:albumArtURI'];
-      const art = (uri as string).replace(/&amp;/gi, '&').replace(/%25/g, '%').replace(/%3a/gi, ':');
+      // Github user @hklages discovered that the album uri sometimes doesn't work because of encoding:
+      // See https://github.com/svrooij/node-sonos-ts/issues/93 if you found and album art uri that doesn't work.
+      const art = (uri as string).replace(/&amp;/gi, '&'); // .replace(/%25/g, '%').replace(/%3a/gi, ':');
       track.AlbumArtUri = art.startsWith('http') ? art : `http://${host}:${port}${art}`;
     }
 

diff --git a/src/helpers/xml-helper.ts b/src/helpers/xml-helper.ts
@@ -1,8 +1,10 @@
 import { parse } from 'fast-xml-parser';
-import { XmlEntities } from 'html-entities';
+import { XmlEntities, AllHtmlEntities } from 'html-entities';
 
 export default class XmlHelper {
-  private static entities = new XmlEntities();
+  private static xmlEntities = new XmlEntities();
+
+  private static htmlEntities = new AllHtmlEntities();
 
   /**
    * Decode an encoded xml string
@@ -17,7 +19,23 @@ export default class XmlHelper {
       return undefined;
     }
 
-    return XmlHelper.entities.decode(text);
+    return XmlHelper.xmlEntities.decode(text);
+  }
+
+  /**
+   * Decode an encoded xml string
+   *
+   * @static
+   * @param {string} text Encoded XML
+   * @returns {string} Decoded XML
+   * @memberof XmlHelper
+   */
+  static DecodeHtml(text: unknown): string | undefined {
+    if (typeof text !== 'string' || text === '') {
+      return undefined;
+    }
+
+    return XmlHelper.htmlEntities.decode(text);
   }
 
   /**
@@ -57,15 +75,15 @@ export default class XmlHelper {
    */
   static EncodeXml(xml: unknown): string {
     if (typeof xml !== 'string' || xml === '') return '';
-    return XmlHelper.entities.encode(xml);
+    return XmlHelper.xmlEntities.encode(xml);
   }
 
   static EncodeXmlUndefined(xml: unknown): string | undefined {
     if (typeof xml === 'undefined') {
       return undefined;
     }
     if (typeof xml === 'string') {
-      return xml === '' ? undefined : XmlHelper.entities.encode(xml);
+      return xml === '' ? undefined : XmlHelper.xmlEntities.encode(xml);
     }
 
     return XmlHelper.EncodeXml(`${xml}`);

diff --git a/tests/helpers/metadata-helper.test.ts b/tests/helpers/metadata-helper.test.ts
@@ -87,7 +87,38 @@ describe('MetadataHelper', () => {
   })
 
   describe('ParseDIDLTrack', () => {
-    it('parsed r:streamContent correctly 2', () => {
+    it('decodes html entities', (done) => {
+      const album = 'Christmas &amp; New Year';
+      const title = 'Bell&apos;s';
+      const didl = {
+        'upnp:album': album,
+        'dc:title': title
+      };
+      const result = MetadataHelper.ParseDIDLTrack(didl, 'fake_host');
+      expect(result).to.has.property('Album','Christmas & New Year');
+      expect(result).to.has.property('Title', 'Bell\'s');
+      done();
+    });
+
+    it('decodes spotify album art uri correctly', (done) => {
+      const hostName = 'fake_host'
+      const albumArt = '/getaa?s=1&amp;u=x-sonos-spotify%3aspotify%253atrack%253a0WS5DKZ6QnHvFYBk8lRRm0%3fsid%3d9%26flags%3d8224%26sn%3d7';
+      const expectedUri = `http://${hostName}:1400/getaa?s=1&u=x-sonos-spotify%3aspotify%253atrack%253a0WS5DKZ6QnHvFYBk8lRRm0%3fsid%3d9%26flags%3d8224%26sn%3d7`;
+      const didl = {
+        'upnp:albumArtURI': albumArt,
+        'upnp:album': 'CeeLo&apos;s Magic Moment', 
+        'dc:creator': 'CeeLo Green',
+        'dc:title': 'This Christmas'
+      };
+      const result = MetadataHelper.ParseDIDLTrack(didl, 'fake_host');
+      expect(result).to.have.property('Album', 'CeeLo\'s Magic Moment');
+      expect(result).to.have.property('AlbumArtUri', expectedUri);
+      expect(result).to.have.property('Artist', 'CeeLo Green');
+      expect(result).to.have.property('Title', 'This Christmas');
+      done();
+    });
+
+    it('parsed r:streamContent correctly 2', (done) => {
       const artist = 'Guus Meeuwis';
       const title = 'Brabant'
       const id = 'FAKE_ITEM_ID'
@@ -100,9 +131,10 @@ describe('MetadataHelper', () => {
       expect(result).to.have.nested.property('Artist', artist);
       expect(result).to.have.nested.property('Title', title);
       expect(result).to.have.nested.property('ItemId', id);
+      done();
     })
 
-    it('parsed r:streamContent and r:radioShowMd correctly', () => {
+    it('parsed r:streamContent and r:radioShowMd correctly', (done) => {
       const artist = 'Guus Meeuwis';
       const title = 'Brabant'
       const id = 'FAKE_ITEM_ID'
@@ -116,9 +148,10 @@ describe('MetadataHelper', () => {
       expect(result).to.have.nested.property('Artist', artist);
       expect(result).to.have.nested.property('Title', title);
       expect(result).to.have.nested.property('ItemId', id);
+      done();
     })
 
-    it('parsed r:streamContent', () => {
+    it('parsed r:streamContent', (done) => {
       const artist = 'Guus Meeuwis';
       const id = 'FAKE_ITEM_ID'
       const didl = {
@@ -129,8 +162,9 @@ describe('MetadataHelper', () => {
       expect(result).to.be.an('object');
       expect(result).to.have.nested.property('Artist', artist);
       expect(result).to.have.nested.property('ItemId', id);
-    })
-  })
+      done();
+    });
+  });
 
   describe('TrackToMetaData', () => {
     it('includes resource', () => {