[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> Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
2025-01-25 14:55:55 +01:00 · 2024-06-27 19:22:29 +02:00 · 2024-06-27 19:22:29 +02:00 · 54d5ad169e
commit 54d5ad169e
parent e345a0f860
4 changed files with 156 additions and 29 deletions
--- 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:
--- 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);
--- 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
@ -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"));
+    }
+}
--- a/bundles/org.openhab.binding.bosesoundtouch/src/test/resources/NowPlayingUpdate.xml
+++ 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>