Skip to content

Fix unbound prefix recovery when xmlns:dlna is absent - #53

Open
sfortis wants to merge 3 commits into
StevenLooman:masterfrom
sfortis:fix/unbound-prefix-without-dlna-namespace
Open

Fix unbound prefix recovery when xmlns:dlna is absent#53
sfortis wants to merge 3 commits into
StevenLooman:masterfrom
sfortis:fix/unbound-prefix-without-dlna-namespace

Conversation

@sfortis

@sfortis sfortis commented Jun 21, 2026

Copy link
Copy Markdown

Summary

The `strict=False` recovery added in #35 anchored its namespace injection on an existing `xmlns:dlna` declaration. Some real-world devices (JBL Authentics, WiiM/LinkPlay) emit `song:*` tags without declaring `xmlns:dlna`, so the injection was silently skipped and the unbound prefix remained, raising `ParseError` when the path was otherwise supposed to recover.

Observable today: `python-didl-lite==1.5.0` is shipped via `async-upnp-client` into Music Assistant, where JBL Authentics 200 owners still see ~100 `unbound prefix` errors per hour and a broken DLNA state-sync loop (downstream tracking: music-assistant/support#4398).

Fix

Anchor the namespace injection on the DIDL-Lite root opening tag (`<DIDL-Lite`) instead of the optional `xmlns:dlna` declaration. The root element is guaranteed to be present in any valid DIDL-Lite document, so recovery works regardless of which namespace declarations the producer chose to include.

Also batches all missing prefix injections into a single regex substitution rather than looping per-prefix.

Test

Added `test_from_xml_string_unbound_prefix_without_dlna_namespace` that exercises the no-`xmlns:dlna` case. The existing `test_from_xml_string_unbound_prefix` (with `xmlns:dlna` present) still passes.

```
$ python3 -m pytest tests/ -v
...
26 passed in 0.18s
```

`ruff check` clean.

Compatibility

  • No behavior change when `strict=True` (recovery gated by `if not strict`).
  • No behavior change for XML that did contain `xmlns:dlna` and had unbound prefixes (Handle unbound XML prefixes when strict=False #35 case continues to work).
  • Recovers an additional class of input that previously raised.

(Replaces #52, which was accidentally opened from the wrong account.)

sfortis and others added 2 commits June 21, 2026 13:37
The strict=False recovery added in StevenLooman#35 anchored its namespace injection
on an existing `xmlns:dlna` declaration. Some real-world devices (JBL
Authentics, WiiM/LinkPlay) emit `<song:*>` tags without declaring
`xmlns:dlna`, so the injection was silently skipped and the unbound
prefix remained, raising ParseError when otherwise recoverable.

Anchor the injection on the DIDL-Lite root opening tag instead. That
element is guaranteed to be present in any valid DIDL-Lite document, so
the recovery works regardless of which namespace declarations the
producer chose to include.

Also batch all missing prefixes into a single regex substitution and add
a regression test that exercises the no-dlna-namespace case.

@StevenLooman StevenLooman left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR @sfortis. Apologies for not replying earlier, I must have missed the notification mail.

Comment thread didl_lite/didl_lite.py Outdated
f'xmlns:{prefix}="http://tempuri.org/{prefix}/"' for prefix in sorted(missing_prefixes)
)
xml_string = re.sub(
r"<DIDL-Lite\b",

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a DIDL-Lite XML can use a namespace prefix. E.g., <dlna:DIDL-Lite .... Then this won't work.

Perhaps it should match on <(\w+:)?DIDL-Lite\b.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a real gap, thanks for catching it. I confirmed it against the branch: a document with
a prefixed root fails with unbound prefix exactly as before the fix, because <DIDL-Lite\b
never matches <didl:DIDL-Lite.

One detail came up while applying your suggestion. Changing only the pattern leaves the
replacement as a fixed string, so the prefix is dropped and the opening tag stops matching its
closing tag. The match therefore has to be captured and written back:

xml_string = re.sub(
    r"<((?:[A-Za-z_][\w.-]*:)?DIDL-Lite)\b",
    lambda match: f"<{match.group(1)} {injections}",
    xml_string,
    count=1,
)

I used a callable rather than a string replacement so that the injected declarations are not run
through the backreference expansion of re.sub.

The prefix class is [A-Za-z_][\w.-]* rather than \w+ because a namespace prefix is an NCName,
which allows -, . and _ after the first character.

I ran the candidates over the same inputs to check both points:

root form        | current branch | pattern only    | captured, \w   | captured, NCName
-----------------|----------------|-----------------|----------------|------------------
<DIDL-Lite>      | pass           | pass            | pass           | pass
<didl:DIDL-Lite> | unbound prefix | mismatched tag  | pass           | pass
<my-ns:DIDL-Lite>| unbound prefix | unbound prefix  | unbound prefix | pass

While checking that, I noticed the two detection regexes above have the same limitation, so a
prefix like <my-song:subTitle> is never even reported as missing and recovery does not run at
all, regardless of the root form. Since this PR is about unbound prefix recovery I made all three
consistent:

used_prefixes = set(re.findall(r"<([A-Za-z_][\w.-]*):", xml_string))
defined_prefixes = set(re.findall(r"xmlns:([A-Za-z_][\w.-]*)=", xml_string))

Happy to split that part into its own PR if you would rather keep this one scoped to the anchor.

Two tests are added, one for the prefixed root and one for prefixes containing -, . and _.
Both fail on the previous code and pass now, and the existing suite is unaffected.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.82%. Comparing base (bc2d7dc) to head (24beb40).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
+ Coverage   94.60%   94.82%   +0.21%     
==========================================
  Files           3        3              
  Lines         408      406       -2     
  Branches       46       45       -1     
==========================================
- Hits          386      385       -1     
  Misses         12       12              
+ Partials       10        9       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@StevenLooman

Copy link
Copy Markdown
Owner

Looks good to me. Can you add a Towncrier fragment, as is required for PRs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants