diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/SoapClient.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/SoapClient.kt index d58e33e5..938f2ef4 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/SoapClient.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/SoapClient.kt @@ -99,7 +99,7 @@ class SoapClient( } else { if (inResponse) { val name = parser.name - val text = runCatching { parser.nextText() }.getOrDefault("") + val text = readElementContent(parser, name) args[name] = text } inResponse @@ -109,6 +109,51 @@ class SoapClient( else -> inResponse } + /** + * Read the content of the currently-started element. Try nextText() first + * (works when the content is text -- escaped XML included). If that throws + * (because the content is nested elements), manually walk to the matching + * END_TAG, accumulating text and re-serializing child elements. + * + * Some Sonos firmware sends the GetZoneGroupState payload as nested XML + * elements without escaping; this fallback recovers that path. + */ + private fun readElementContent(parser: XmlPullParser, tagName: String): String { + return runCatching { parser.nextText() }.getOrElse { + readUntilEndTag(parser, tagName) + } + } + + private fun readUntilEndTag(parser: XmlPullParser, tagName: String): String { + val sb = StringBuilder() + var depth = 1 + while (depth > 0) { + when (parser.next()) { + XmlPullParser.START_TAG -> { + sb.append('<').append(parser.name) + for (i in 0 until parser.attributeCount) { + sb.append(' ') + .append(parser.getAttributeName(i)) + .append("=\"") + .append(parser.getAttributeValue(i)) + .append('"') + } + sb.append('>') + depth += 1 + } + XmlPullParser.END_TAG -> { + depth -= 1 + if (depth == 0 && parser.name == tagName) break + sb.append("') + } + XmlPullParser.TEXT -> sb.append(parser.text.orEmpty()) + XmlPullParser.END_DOCUMENT -> break + else -> Unit + } + } + return sb.toString() + } + private fun faultCodeOf(body: String): String = extractBetween(body, "", "") ?: "unknown" @@ -141,23 +186,32 @@ private const val MAX_BODY_LOG_CHARS = 2048 /** * Returns a [SoapClient] that logs the raw response body for the first - * [MAX_DIAGNOSTIC_RESPONSES] GetPositionInfo / GetTransportInfo calls (i.e. - * the first 3 poll cycles of a UPnP session). After that the callback is a - * no-op so there is no persistent log spam. Logged at WARN so release builds - * capture it without a separate log-level override. + * [MAX_DIAGNOSTIC_RESPONSES] calls for actions in [DIAGNOSTIC_ACTIONS] + * (GetPositionInfo, GetTransportInfo, GetZoneGroupState). After that the + * callback is a no-op so there is no persistent log spam. Logged at WARN + * so release builds capture it without a separate log-level override. */ internal fun loggingSoapClient(okHttp: OkHttpClient, label: String): SoapClient { val counter = AtomicInteger(0) return SoapClient(okHttp) { action, body -> - if (action != "GetPositionInfo" && action != "GetTransportInfo") return@SoapClient + if (action !in DIAGNOSTIC_ACTIONS) return@SoapClient val n = counter.incrementAndGet() if (n <= MAX_DIAGNOSTIC_RESPONSES) { - Timber.w("UPnP %s response #%d (%s): %s", - label, n, action, body.take(MAX_BODY_LOG_CHARS)) + Timber.w( + "UPnP %s response #%d (%s): %s", + label, n, action, + body.take(MAX_BODY_LOG_CHARS), + ) } } } +private val DIAGNOSTIC_ACTIONS = setOf( + "GetPositionInfo", + "GetTransportInfo", + "GetZoneGroupState", +) + /** XML-escapes a string value for embedding as text content inside a SOAP envelope. */ internal fun xmlEscape(v: String): String = v .replace("&", "&") diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpDiscoveryController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpDiscoveryController.kt index 0cb06f15..b43da397 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpDiscoveryController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/UpnpDiscoveryController.kt @@ -125,7 +125,10 @@ class UpnpDiscoveryController @Inject constructor( return } val groups = runCatching { - ZoneGroupTopologyClient(SoapClient(okHttp), zgtUrl).getZoneGroupState() + ZoneGroupTopologyClient( + loggingSoapClient(okHttp, "ZGT-${anySonos.id}"), + zgtUrl, + ).getZoneGroupState() } .onFailure { Timber.w(it, "refreshSonosTopology failed for %s", anySonos.id) } .getOrNull() ?: return diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/sonos/SonosTopology.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/sonos/SonosTopology.kt index 46ef494e..56d276f1 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/sonos/SonosTopology.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/sonos/SonosTopology.kt @@ -24,9 +24,21 @@ data class SonosZoneMember( object SonosTopology { fun parse(xml: String): List { - return runCatching { parseStrict(xml) }.getOrDefault(emptyList()) + val effective = if (xml.contains("<ZoneGroup")) { + unescapeXmlEntities(xml) + } else { + xml + } + return runCatching { parseStrict(effective) }.getOrDefault(emptyList()) } + private fun unescapeXmlEntities(s: String): String = s + .replace("<", "<") + .replace(">", ">") + .replace(""", "\"") + .replace("'", "'") + .replace("&", "&") // must be last to avoid double-decoding + private fun parseStrict(xml: String): List { val parser = XmlPullParserFactory.newInstance().newPullParser().apply { setInput(xml.reader()) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/sonos/ZoneGroupTopologyClient.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/sonos/ZoneGroupTopologyClient.kt index 684b7389..35cc0665 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/sonos/ZoneGroupTopologyClient.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/upnp/sonos/ZoneGroupTopologyClient.kt @@ -2,6 +2,7 @@ package com.fabledsword.minstrel.player.output.upnp.sonos import com.fabledsword.minstrel.player.output.upnp.SoapClient import okhttp3.HttpUrl +import timber.log.Timber /** * Sonos's proprietary ZoneGroupTopology service. Same SOAP shape as a @@ -21,10 +22,16 @@ class ZoneGroupTopologyClient( args = emptyMap(), ) val inner = args["ZoneGroupState"].orEmpty() + Timber.w( + "ZGT extracted ZoneGroupState (%d chars): %s", + inner.length, + inner.take(ZGT_LOG_TRUNCATE_CHARS), + ) return SonosTopology.parse(inner) } private companion object { const val SERVICE_TYPE = "urn:schemas-upnp-org:service:ZoneGroupTopology:1" + const val ZGT_LOG_TRUNCATE_CHARS = 2048 } } diff --git a/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/sonos/SonosTopologyTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/sonos/SonosTopologyTest.kt index 2c5511f6..23caef85 100644 --- a/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/sonos/SonosTopologyTest.kt +++ b/android/app/src/test/java/com/fabledsword/minstrel/player/output/upnp/sonos/SonosTopologyTest.kt @@ -71,4 +71,39 @@ class SonosTopologyTest { fun `malformed xml returns empty list`() { assertEquals(emptyList(), SonosTopology.parse(" + + + + + + + """.trimIndent() + val groups = SonosTopology.parse(xml) + assertEquals(1, groups.size) + } + + @Test + fun `parses escaped ZoneGroupState wrapped in entities`() { + val xml = """ + <ZoneGroupState> + <ZoneGroups> + <ZoneGroup Coordinator="RINCON_A" ID="RINCON_A:1"> + <ZoneGroupMember UUID="RINCON_A" ZoneName="Kitchen" + Location="http://192.168.1.10:1400/xml/device_description.xml"/> + </ZoneGroup> + </ZoneGroups> + </ZoneGroupState> + """.trimIndent() + val groups = SonosTopology.parse(xml) + assertEquals(1, groups.size) + } }