fix(metadata): lenient RDFa extraction for unbound XML prefixes (#4) - #37
Conversation
natechadwick-intsof
left a comment
There was a problem hiding this comment.
Summary
This is a solid, JDK 8-compatible backport of the already-merged sister fix (percussioncms#2375). PSMetadataExtractorService now unwraps undeclared XML-style prefixed elements/attributes (the gcse:search case) before XHTML serialization and, if SAX still reports an unbound prefix, swallows that parse failure so DTS metadata delivery no longer ERRORs the whole page. The strip logic matches Jsoup 1.23 (tagName() is the qualified name, e.g. gcse:search), reserved xml/xmlns prefixes are kept, and dcterms/og values live in unprefixed attribute values so they are not stripped. Dominant residual risk is that the new fixture places every asserted RDFa node before the unbound markup, so a broken sanitizer plus the new catch would still go green; the fallback matcher is also a bit broader than the sister PR.
Issue counts by severity
- bugs: 0
- suggestions: 2
- nits: 0
…fter-markup test) Review feedback on PR #37 (natechadwick-intsof): 1) testUnboundPrefixGcseSearch could not tell a working strip from a failed strip + parse catch. All asserted dcterms:* meta tags sat BEFORE the unbound markup, so even a no-op sanitizer + the catch would still produce a green test. Move the dcterms:source meta to AFTER the gcse:search / vendor:data-id in unbound-prefix-gcse.html so the assertion can only succeed if the sanitizer actually ran. Also add a direct unit test (testStripUnboundPrefixedMarkupRemovesGcseAndVendorButKeepsDcterms) that drives stripUnboundPrefixedMarkup directly and walks the Jsoup tree to assert gcse:search / vendor:data-id are gone, a declared xmlns:foo element survives, and dcterms:* metadata is intact. 2) isUnboundPrefixParseFailure previously trusted a bare prefix "x" for element|attribute pattern, which could swallow non-unbound SAX diagnostics that happen to mention the same words and silently drop every subsequent RDFa triple on the page. Tighten the detector: only SAXParseException (or SAX-typed throwable) + a message containing 'not bound' is treated as ignorable. The looser UNBOUND_PREFIX_MESSAGE pattern is now only used by extractUnboundPrefix(Throwable) for WARN log labeling -- never to decide whether to swallow a parse error. Update the existing testIsUnboundPrefixParseFailureDetectsGcse to use a real SAXParseException, and add testIsUnboundPrefixParseFailureIgnoresUntypedThrowableWithSameMessage (negative) and testIsUnboundPrefixParseFailureIgnoresUnboundPrefixInCauseChainOfUntypedThrowable (cause-chain positive). Bump stripUnboundPrefixedMarkup visibility from package-private to public so it can be tested from com.percussion.delivery (the test package lives one level above com.percussion.delivery.metadata). Verification: 12/12 tests pass.
Pre-sanitize published HTML in PSMetadataExtractorService before RDFa
parse so vendor embeds like Google CSE <gcse:search> without an xmlns:gcse
declaration no longer throw SAXParseException and fail the whole page's
metadata delivery. Collect xmlns:* declarations on the document, strip
elements/attributes whose prefixes are not declared (deepest-first via
Jsoup.unwrap), and rewrite non-XML named entities to numeric character
references. As a defensive fallback the RDFa parse is wrapped in a
try/catch that detects unbound-prefix parse messages and logs WARN with
the page path and offending prefix so non-RDFa fields still flow through.
dcterms:*/og:*/perc:* metadata is unaffected because the metadata of
interest lives in attribute values, not in unbound element/attribute names.
Adds:
- PSMetadataExtractorService.stripUnboundPrefixedMarkup(Document, String)
- PSMetadataExtractorService.isUnboundPrefixParseFailure(Throwable)
- PSMetadataExtractorService.extractUnboundPrefix(Throwable)
- tests: testUnboundPrefixGcseSearch, testUnboundPrefixOnlyDoesNotThrow,
testIsUnboundPrefixParseFailureDetectsGcse,
testIsUnboundPrefixParseFailureIgnoresOtherErrors
- fixture: system/UnitTestResources/com/percussion/delivery/unbound-prefix-gcse.html
- CHANGELOG entry under 8.1.8 / GH_POST_PR_COMMIT_RUN_ID placeholder
Also updates AGENTS.md to point at the main branch (not development-8.1.x).
Signed-off-by: Vijaya Boddipudi <216913149+vijaya-boddipudi@users.noreply.github.com>
…fter-markup test) Review feedback on PR #37 (natechadwick-intsof): 1) testUnboundPrefixGcseSearch could not tell a working strip from a failed strip + parse catch. All asserted dcterms:* meta tags sat BEFORE the unbound markup, so even a no-op sanitizer + the catch would still produce a green test. Move the dcterms:source meta to AFTER the gcse:search / vendor:data-id in unbound-prefix-gcse.html so the assertion can only succeed if the sanitizer actually ran. Also add a direct unit test (testStripUnboundPrefixedMarkupRemovesGcseAndVendorButKeepsDcterms) that drives stripUnboundPrefixedMarkup directly and walks the Jsoup tree to assert gcse:search / vendor:data-id are gone, a declared xmlns:foo element survives, and dcterms:* metadata is intact. 2) isUnboundPrefixParseFailure previously trusted a bare prefix "x" for element|attribute pattern, which could swallow non-unbound SAX diagnostics that happen to mention the same words and silently drop every subsequent RDFa triple on the page. Tighten the detector: only SAXParseException (or SAX-typed throwable) + a message containing 'not bound' is treated as ignorable. The looser UNBOUND_PREFIX_MESSAGE pattern is now only used by extractUnboundPrefix(Throwable) for WARN log labeling -- never to decide whether to swallow a parse error. Update the existing testIsUnboundPrefixParseFailureDetectsGcse to use a real SAXParseException, and add testIsUnboundPrefixParseFailureIgnoresUntypedThrowableWithSameMessage (negative) and testIsUnboundPrefixParseFailureIgnoresUnboundPrefixInCauseChainOfUntypedThrowable (cause-chain positive). Bump stripUnboundPrefixedMarkup visibility from package-private to public so it can be tested from com.percussion.delivery (the test package lives one level above com.percussion.delivery.metadata). Verification: 12/12 tests pass. Signed-off-by: Vijaya Boddipudi <216913149+vijaya-boddipudi@users.noreply.github.com>
5b4e53c to
d194e6c
Compare
Fixes #4
Summary
PSMetadataExtractorService(RDFa / Semargl parse over published HTML) previously threwSAXParseException: The prefix "gcse" for element "gcse:search" is not boundwhen published pages included vendor embeds such as Google Custom Search without anxmlns:gcsedeclaration. That failure aborted the whole page's metadata delivery inPSMetadataDeliveryHandlerwith ERROR even when file publish itself succeeded.This change makes extraction lenient:
xmlns:*declarations on the document, strip elements and attributes whose prefixes are not declared (deepest-first viaJsoup.unwrap, so children move to the parent before the parent is unwrapped), and rewrite non-XML named entities to numeric character references.SAXParseException/ unbound-prefix parse messages and logs WARN with the page path and the offending prefix when extractable, so the rest of the page metadata still flows through.dcterms:*/og:*/perc:*metadata is unchanged because the metadata of interest lives in attribute values, not in unbound element/attribute names.Acceptance criteria
gcse:search(and similar) does not throw out ofPSMetadataExtractorService/ fail the delivery handler with ERROR for that reason alone.dctermsmetadata still extracted when present (verified by the newtestUnboundPrefixGcseSearchfixture, which also assertsdcterms:source,dcterms:title,dcterms:description,dcterms:abstract).gcse(and generic unbound prefix) fixtures.Files changed
system/business/src/com/percussion/delivery/metadata/PSMetadataExtractorService.java— pre-sanitize (stripUnboundPrefixedMarkup,rewriteNonXmlNamedEntities) + lenient catch around RDFa parse (isUnboundPrefixParseFailure,extractUnboundPrefix).system/Testing/src/com/percussion/delivery/PSMetadataExtractorServiceTests.java— 4 new tests (full fixture, minimal inline HTML, positive / negative helper coverage).system/UnitTestResources/com/percussion/delivery/unbound-prefix-gcse.html— new fixture with<gcse:search>,vendor:data-idattribute, anddcterms:*/og:*metadata.CHANGELOG.md— new 8.1.8 /GH_POST_PR_COMMIT_RUN_IDentry perAGENTS.mdguidelines.AGENTS.md— point atmain(the active branch), not the non-existentdevelopment-8.1.x.Verification
./mvn-env.sh test -pl system -am -Dtest=PSMetadataExtractorServiceTests -Dsurefire.failIfNoSpecifiedTests=falseSpotless check on the modified files: clean. Checkstyle: pre-existing config issue on
main(JavadocMethod.scopeproperty) — not introduced by this change.Related