slachiewicz commented on PR #898: URL: https://github.com/apache/maven-wagon/pull/898#issuecomment-5221839968
Pushed a third commit addressing review feedback. Four fixes, all in the new code: **The parser could hard-break the transport.** `disallow-doctype-decl` is a Xerces feature, and `setFeature` throws for features a parser does not know. `DocumentBuilderFactory.newInstance()` is overridable via system property and `ServiceLoader`, and Maven runs with plugin and extension classpaths that can carry alternate JAXP implementations — so a non-platform parser would have made *every* PROPFIND fail. Worse, the exception was caught alongside `SAXException` and reported as `Cannot parse multistatus response`, blaming a server that had answered perfectly. The feature is now applied only where supported, falling back to refusing external general and parameter entities, and a configuration failure is reported as one. **No `ErrorHandler` on the `DocumentBuilder`.** Without one the parser writes `[Fatal Error] :1:1: ...` to stderr before throwing. A server answering 207 with an HTML error page would have littered the build output on top of the wrapped exception. Verified the noise and confirmed it is gone. **A 207 with no body was downgraded to a missing resource.** It was read as "not a collection", which surfaced as `ResourceDoesNotExistException` — telling the resolver the artifact is simply absent, when in fact the transport misbehaved. It is now an `IOException`, so it reaches callers as `TransferFailedException` as it did before. **Comment corrected on repeated hrefs.** It claimed later duplicates were dropped; `LinkedHashMap.put` in fact lets the last win at the first one's position. That matches what the Jackrabbit-backed code did, so the behaviour stands and the comment was wrong. The test now pins which entry wins rather than only asserting the count. Two deliberate semantic differences from the Jackrabbit version, now recorded in javadoc and covered by tests: - A `propstat` without a `DAV:status` is read as successful. RFC 4918 requires the element; Jackrabbit skipped any `propstat` lacking it, which made every property of such a response invisible. - A DOCTYPE is refused outright rather than neutered by an `EntityResolver`. 293 tests pass. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
