From 33a9894bde0e8492cff6afaff18c8b7e553ed769 Mon Sep 17 00:00:00 2001 From: David Pace Date: Thu, 27 Jun 2024 19:22:29 +0200 Subject: [PATCH] [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 --- .../internal/XMLResponseHandler.java | 90 +++++++++++++------ .../internal/XMLResponseProcessor.java | 1 + .../internal/XMLResponseProcessorTest.java | 74 +++++++++++++++ .../src/test/resources/NowPlayingUpdate.xml | 20 +++++ 4 files changed, 156 insertions(+), 29 deletions(-) create mode 100644 bundles/org.openhab.binding.bosesoundtouch/src/test/java/org/openhab/binding/bosesoundtouch/internal/XMLResponseProcessorTest.java create mode 100644 bundles/org.openhab.binding.bosesoundtouch/src/test/resources/NowPlayingUpdate.xml 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 e4d539e1646..8990ef062fe 100644 --- 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 playerPresets; + /** + * String builder to collect text content. + *

+ * Background: {@code characters()} may be called multiple times for the same + * text content in case there are entities like {@code '} 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 fdc89fb8d16..36d2ed8a019 100644 --- 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 index 00000000000..9ac532660e9 --- /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 index 00000000000..1456aed8c7b --- /dev/null +++ b/bundles/org.openhab.binding.bosesoundtouch/src/test/resources/NowPlayingUpdate.xml @@ -0,0 +1,20 @@ + + + + + iPhone + + Sweet Child O' Mine + Guns N' Roses + "Appetite for Destruction" + Jammin' + All o'er the world + + + PLAY_STATE + + Rock 'n' Roll + + + + \ No newline at end of file