mattcasters commented on code in PR #8368:
URL: https://github.com/apache/hop/pull/8368#discussion_r4008381650
##########
docs/hop-user-manual/modules/ROOT/pages/pipeline/transforms/xsdvalidator.adoc:
##########
@@ -71,6 +71,8 @@ See also the xref:workflow/actions/xsdvalidator.adoc[XSD
Validator workflow acti
* Add validation msg in output : check this if you want to add a validation
message in the output.
* Validation msg field: the name of the validation message field.
Review Comment:
**[suggestion]** The new docs treat the checkbox as an XXE switch ("resolve
external entities (XXE)") and tell operators to leave it unchecked unless
schemas "need to load external entities." That undersells what flag=false
actually does, which the PR body already admits: "XSD source is defined inside
the XML" stops working. That is not the new resolver being strict about `http:`
— it is the pre-existing Validator `XMLEntityResolver` that throws on every
resource, so even a local `xsi:schemaLocation` fails. Hop's own
`0002-xsd-validator-test` / `main-0003-xsd-validator-action-test` only pass
because they serialize `allowExternalEntities=Y` and point `xsi:schemaLocation`
at `http://file-server:8080/test.xsd`. New transforms/actions will hit this
immediately (the action docs still list "none (default)" / "is defined inside
XML" as a first-class source). The same checkbox is also what re-enables remote
`xs:include`/`xs:import` (including the common W3C `xml.xsd` HTTP import). Local
file `xs:include` does **not** need the checkbox; the new factory resolver
allows `file:` / relative includes, and `SecureSchemaFactoryTest` covers that.
**Suggestion:** On this row (and the matching sentence in
`docs/hop-user-manual/modules/ROOT/pages/workflow/actions/xsdvalidator.adoc:43`)
state the three effects of the default: (1) instance-document DOCTYPE/XXE is
refused; (2) "XSD is defined inside the XML" requires the checkbox, including
for a local schemaLocation; (3) remote schema include/import requires the
checkbox, local file includes do not. Keep the system-property note.
--
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]
