[bosesoundtouch] Fix parsing of metadata fields (#16898)

XML text content was not processed correctly in case the value contained XML entities.
Signed-off-by: David Pace <dev@davidpace.de>

diff --git a/bundles/org.openhab.binding.bosesoundtouch/src/main/java/org/openhab/binding/bosesoundtouch/internal/XMLResponseHandler.java b/bundles/org.openhab.binding.bosesoundtouch/src/main/java/org/openhab/binding/bosesoundtouch/internal/XMLResponseHandler.java
index e4d539e1646b12c51dfb76395e6694b94246a8a4..8990ef062fedf8d49a3a9f2f0396176fa1d55fcf 100644 (file)
--- a/bundles/org.openhab.binding.bosesoundtouch/src/main/java/org/openhab/binding/bosesoundtouch/internal/XMLResponseHandler.java
+++ b/bundles/org.openhab.binding.bosesoundtouch/src/main/java/org/openhab/binding/bosesoundtouch/internal/XMLResponseHandler.java
@@ -72,6 +72,15 @@ public class XMLResponseHandler extends DefaultHandler {
 
     private Map<Integer, ContentItem> playerPresets;
 
+    /**
+     * String builder to collect text content.
+     * <p>
+     * Background: {@code characters()} may be called multiple times for the same
+     * text content in case there are entities like {@code &apos;} inside the
+     * content.
+     */
+    private StringBuilder textContent = new StringBuilder();
+
     /**
      * Creates a new instance of this class
      *
@@ -96,6 +105,7 @@ public class XMLResponseHandler extends DefaultHandler {
         state = XMLHandlerState.Unprocessed; // set default value; we avoid default in select to have the compiler
                                              // showing a
                                              // warning for unhandled states
+        textContent = new StringBuilder();
 
         switch (curState) {
             case INIT:
@@ -475,6 +485,27 @@ public class XMLResponseHandler extends DefaultHandler {
             case Group:
                 handler.handleGroupUpdated(masterDeviceId);
                 break;
+            case NowPlayingAlbum:
+                updateNowPlayingAlbum(new StringType(textContent.toString()));
+                break;
+            case NowPlayingArtist:
+                updateNowPlayingArtist(new StringType(textContent.toString()));
+                break;
+            case NowPlayingDescription:
+                updateNowPlayingDescription(new StringType(textContent.toString()));
+                break;
+            case NowPlayingGenre:
+                updateNowPlayingGenre(new StringType(textContent.toString()));
+                break;
+            case NowPlayingStationLocation:
+                updateNowPlayingStationLocation(new StringType(textContent.toString()));
+                break;
+            case NowPlayingStationName:
+                updateNowPlayingStationName(new StringType(textContent.toString()));
+                break;
+            case NowPlayingTrack:
+                updateNowPlayingTrack(new StringType(textContent.toString()));
+                break;
             default:
                 // no actions...
                 break;
@@ -483,8 +514,11 @@ public class XMLResponseHandler extends DefaultHandler {
 
     @Override
     public void characters(char[] ch, int start, int length) throws SAXException {
-        logger.trace("{}: Text data during {}: '{}'", handler.getDeviceName(), state, new String(ch, start, length));
+        String string = new String(ch, start, length);
+        logger.trace("{}: Text data during {}: '{}'", handler.getDeviceName(), state, string);
+
         super.characters(ch, start, length);
+
         switch (state) {
             case INIT:
             case Msg:
@@ -507,8 +541,7 @@ public class XMLResponseHandler extends DefaultHandler {
             case Zone:
             case ZoneUpdated:
             case Sources:
-                logger.debug("{}: Unexpected text data during {}: '{}'", handler.getDeviceName(), state,
-                        new String(ch, start, length));
+                logger.debug("{}: Unexpected text data during {}: '{}'", handler.getDeviceName(), state, string);
                 break;
             case BassMin: // @TODO - find out how to dynamically change "channel-type" bass configuration
             case BassMax: // based on these values...
@@ -518,38 +551,37 @@ public class XMLResponseHandler extends DefaultHandler {
                 // this are currently unprocessed values.
                 break;
             case BassCapabilities:
-                logger.debug("{}: Unexpected text data during {}: '{}'", handler.getDeviceName(), state,
-                        new String(ch, start, length));
+                logger.debug("{}: Unexpected text data during {}: '{}'", handler.getDeviceName(), state, string);
                 break;
             case Unprocessed:
                 // drop quietly..
                 break;
             case BassActual:
-                commandExecutor.updateBassLevelGUIState(new DecimalType(new String(ch, start, length)));
+                commandExecutor.updateBassLevelGUIState(new DecimalType(string));
                 break;
             case InfoName:
-                setConfigOption(DEVICE_INFO_NAME, new String(ch, start, length));
+                setConfigOption(DEVICE_INFO_NAME, string);
                 break;
             case InfoType:
-                setConfigOption(DEVICE_INFO_TYPE, new String(ch, start, length));
-                setConfigOption(PROPERTY_MODEL_ID, new String(ch, start, length));
+                setConfigOption(DEVICE_INFO_TYPE, string);
+                setConfigOption(PROPERTY_MODEL_ID, string);
                 break;
             case InfoModuleType:
-                setConfigOption(PROPERTY_HARDWARE_VERSION, new String(ch, start, length));
+                setConfigOption(PROPERTY_HARDWARE_VERSION, string);
                 break;
             case InfoFirmwareVersion:
-                String[] fwVersion = new String(ch, start, length).split(" ");
+                String[] fwVersion = string.split(" ");
                 setConfigOption(PROPERTY_FIRMWARE_VERSION, fwVersion[0]);
                 break;
             case BassAvailable:
-                boolean bassAvailable = Boolean.parseBoolean(new String(ch, start, length));
+                boolean bassAvailable = Boolean.parseBoolean(string);
                 commandExecutor.setBassAvailable(bassAvailable);
                 break;
             case NowPlayingAlbum:
-                updateNowPlayingAlbum(new StringType(new String(ch, start, length)));
+                textContent.append(string);
                 break;
             case NowPlayingArt:
-                String url = new String(ch, start, length);
+                String url = string;
                 if (url.startsWith("http")) {
                     // We download the cover art in a different thread to not delay the other operations
                     handler.getScheduler().submit(() -> {
@@ -565,22 +597,22 @@ public class XMLResponseHandler extends DefaultHandler {
                 }
                 break;
             case NowPlayingArtist:
-                updateNowPlayingArtist(new StringType(new String(ch, start, length)));
+                textContent.append(string);
                 break;
             case ContentItemItemName:
-                contentItem.setItemName(new String(ch, start, length));
+                contentItem.setItemName(string);
                 break;
             case ContentItemContainerArt:
-                contentItem.setContainerArt(new String(ch, start, length));
+                contentItem.setContainerArt(string);
                 break;
             case NowPlayingDescription:
-                updateNowPlayingDescription(new StringType(new String(ch, start, length)));
+                textContent.append(string);
                 break;
             case NowPlayingGenre:
-                updateNowPlayingGenre(new StringType(new String(ch, start, length)));
+                textContent.append(string);
                 break;
             case NowPlayingPlayStatus:
-                String playPauseState = new String(ch, start, length);
+                String playPauseState = string;
                 if ("PLAY_STATE".equals(playPauseState) || "BUFFERING_STATE".equals(playPauseState)) {
                     commandExecutor.updatePlayerControlGUIState(PlayPauseType.PLAY);
                 } else if ("STOP_STATE".equals(playPauseState) || "PAUSE_STATE".equals(playPauseState)) {
@@ -588,37 +620,37 @@ public class XMLResponseHandler extends DefaultHandler {
                 }
                 break;
             case NowPlayingStationLocation:
-                updateNowPlayingStationLocation(new StringType(new String(ch, start, length)));
+                textContent.append(string);
                 break;
             case NowPlayingStationName:
-                updateNowPlayingStationName(new StringType(new String(ch, start, length)));
+                textContent.append(string);
                 break;
             case NowPlayingTrack:
-                updateNowPlayingTrack(new StringType(new String(ch, start, length)));
+                textContent.append(string);
                 break;
             case VolumeActual:
-                commandExecutor.updateVolumeGUIState(new PercentType(Integer.parseInt(new String(ch, start, length))));
+                commandExecutor.updateVolumeGUIState(new PercentType(Integer.parseInt(string)));
                 break;
             case VolumeMuteEnabled:
-                volumeMuteEnabled = Boolean.parseBoolean(new String(ch, start, length));
+                volumeMuteEnabled = Boolean.parseBoolean(string);
                 commandExecutor.setCurrentMuted(volumeMuteEnabled);
                 break;
             case MasterDeviceId:
                 if (masterDeviceId != null) {
-                    masterDeviceId.macAddress = new String(ch, start, length);
+                    masterDeviceId.macAddress = string;
                 }
                 break;
             case GroupName:
                 if (masterDeviceId != null) {
-                    masterDeviceId.groupName = new String(ch, start, length);
+                    masterDeviceId.groupName = string;
                 }
                 break;
             case DeviceId:
-                deviceId = new String(ch, start, length);
+                deviceId = string;
                 break;
             case DeviceIp:
                 if (masterDeviceId != null && Objects.equals(masterDeviceId.macAddress, deviceId)) {
-                    masterDeviceId.host = new String(ch, start, length);
+                    masterDeviceId.host = string;
                 }
                 break;
             default:

diff --git a/bundles/org.openhab.binding.bosesoundtouch/src/main/java/org/openhab/binding/bosesoundtouch/internal/XMLResponseProcessor.java b/bundles/org.openhab.binding.bosesoundtouch/src/main/java/org/openhab/binding/bosesoundtouch/internal/XMLResponseProcessor.java
index fdc89fb8d16935e5f838ca40f39a3f322f7d84ce..36d2ed8a019a10200e3c4e91075ffbfcf70fa1eb 100644 (file)
--- a/bundles/org.openhab.binding.bosesoundtouch/src/main/java/org/openhab/binding/bosesoundtouch/internal/XMLResponseProcessor.java
+++ b/bundles/org.openhab.binding.bosesoundtouch/src/main/java/org/openhab/binding/bosesoundtouch/internal/XMLResponseProcessor.java
@@ -123,6 +123,7 @@ public class XMLResponseProcessor {
         nowPlayingMap.put("artist", XMLHandlerState.NowPlayingArtist);
         nowPlayingMap.put("ContentItem", XMLHandlerState.ContentItem);
         nowPlayingMap.put("description", XMLHandlerState.NowPlayingDescription);
+        nowPlayingMap.put("genre", XMLHandlerState.NowPlayingGenre);
         nowPlayingMap.put("playStatus", XMLHandlerState.NowPlayingPlayStatus);
         nowPlayingMap.put("rateEnabled", XMLHandlerState.NowPlayingRateEnabled);
         nowPlayingMap.put("skipEnabled", XMLHandlerState.NowPlayingSkipEnabled);

diff --git a/bundles/org.openhab.binding.bosesoundtouch/src/test/java/org/openhab/binding/bosesoundtouch/internal/XMLResponseProcessorTest.java b/bundles/org.openhab.binding.bosesoundtouch/src/test/java/org/openhab/binding/bosesoundtouch/internal/XMLResponseProcessorTest.java
new file mode 100644 (file)
index 0000000..9ac5326
--- /dev/null
+++ b/bundles/org.openhab.binding.bosesoundtouch/src/test/java/org/openhab/binding/bosesoundtouch/internal/XMLResponseProcessorTest.java
@@ -0,0 +1,74 @@
+/**
+ * Copyright (c) 2010-2024 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.binding.bosesoundtouch.internal;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.openhab.binding.bosesoundtouch.internal.BoseSoundTouchBindingConstants.CHANNEL_NOWPLAYING_ALBUM;
+import static org.openhab.binding.bosesoundtouch.internal.BoseSoundTouchBindingConstants.CHANNEL_NOWPLAYING_ARTIST;
+import static org.openhab.binding.bosesoundtouch.internal.BoseSoundTouchBindingConstants.CHANNEL_NOWPLAYING_GENRE;
+import static org.openhab.binding.bosesoundtouch.internal.BoseSoundTouchBindingConstants.CHANNEL_NOWPLAYING_STATIONLOCATION;
+import static org.openhab.binding.bosesoundtouch.internal.BoseSoundTouchBindingConstants.CHANNEL_NOWPLAYING_STATIONNAME;
+import static org.openhab.binding.bosesoundtouch.internal.BoseSoundTouchBindingConstants.CHANNEL_NOWPLAYING_TRACK;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+
+import javax.xml.parsers.ParserConfigurationException;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.openhab.binding.bosesoundtouch.internal.handler.BoseSoundTouchHandler;
+import org.openhab.core.library.types.StringType;
+import org.xml.sax.SAXException;
+
+/**
+ * Unit tests for {@link XMLResponseProcessor}.
+ * 
+ * @author David Pace - Initial contribution
+ *
+ */
+@NonNullByDefault
+class XMLResponseProcessorTest {
+
+    private @NonNullByDefault({}) XMLResponseProcessor fixture;
+    private @NonNullByDefault({}) BoseSoundTouchHandler handler;
+
+    @BeforeEach
+    protected void setUp() throws Exception {
+        handler = mock(BoseSoundTouchHandler.class);
+        when(handler.getMacAddress()).thenReturn("5065834D198B");
+
+        CommandExecutor commandExecutor = mock(CommandExecutor.class);
+        when(handler.getCommandExecutor()).thenReturn(commandExecutor);
+
+        fixture = new XMLResponseProcessor(handler);
+    }
+
+    @Test
+    void testParseNowPlayingUpdate() throws SAXException, IOException, ParserConfigurationException {
+        String updateXML = Files.readString(new File("src/test/resources/NowPlayingUpdate.xml").toPath());
+
+        fixture.handleMessage(updateXML);
+
+        verify(handler).updateState(CHANNEL_NOWPLAYING_ALBUM, new StringType("\"Appetite for Destruction\""));
+        verify(handler).updateState(CHANNEL_NOWPLAYING_ARTIST, new StringType("Guns N' Roses"));
+        verify(handler).updateState(CHANNEL_NOWPLAYING_TRACK, new StringType("Sweet Child O' Mine"));
+        verify(handler).updateState(CHANNEL_NOWPLAYING_GENRE, new StringType("Rock 'n' Roll"));
+        verify(handler).updateState(CHANNEL_NOWPLAYING_STATIONNAME, new StringType("Jammin'"));
+        verify(handler).updateState(CHANNEL_NOWPLAYING_STATIONLOCATION, new StringType("All o'er the world"));
+    }
+}

diff --git a/bundles/org.openhab.binding.bosesoundtouch/src/test/resources/NowPlayingUpdate.xml b/bundles/org.openhab.binding.bosesoundtouch/src/test/resources/NowPlayingUpdate.xml
new file mode 100644 (file)
index 0000000..1456aed
--- /dev/null
+++ b/bundles/org.openhab.binding.bosesoundtouch/src/test/resources/NowPlayingUpdate.xml
@@ -0,0 +1,20 @@
+<updates deviceID="5065834D198B">
+    <nowPlayingUpdated>
+        <nowPlaying deviceID="5065834D198B" source="BLUETOOTH" sourceAccount="">
+            <ContentItem source="BLUETOOTH" location="" sourceAccount="" isPresetable="false">
+                <itemName>iPhone</itemName>
+            </ContentItem>
+            <track>Sweet Child O&apos; Mine</track>
+            <artist>Guns N&apos; Roses</artist>
+            <album>&quot;Appetite for Destruction&quot;</album>
+            <stationName>Jammin&apos;</stationName>
+            <stationLocation>All o&apos;er the world</stationLocation>
+            <art artImageStatus="SHOW_DEFAULT_IMAGE" />
+            <skipEnabled />
+            <playStatus>PLAY_STATE</playStatus>
+            <skipPreviousEnabled />
+            <genre>Rock &apos;n&apos; Roll</genre>
+            <connectionStatusInfo status="CONNECTED" deviceName="iPhone" />
+        </nowPlaying>
+    </nowPlayingUpdated>
+</updates>
\ No newline at end of file