You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hop.apache.org by "hansva (via GitHub)" <gi...@apache.org> on 2026/09/23 11:26:10 UTC
[PR] fix wdsl file load, fixes #4813 (hop)
hansva opened a new pull request, #8552:
URL: https://github.com/apache/hop/pull/8552
**Please** add a meaningful description for your change here
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Run `mvn clean install apache-rat:check` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
- [ ] If you have a group of commits related to the same change, please squash your commits into one and force push your branch using `git rebase -i`.
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable.
To make clear that you license your contribution under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [ ] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix wdsl file load, fixes #4813 (hop)
Posted by "hansva (via GitHub)" <gi...@apache.org>.
hansva merged PR #8552:
URL: https://github.com/apache/hop/pull/8552
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix wdsl file load, fixes #4813 (hop)
Posted by "mattcasters (via GitHub)" <gi...@apache.org>.
mattcasters commented on code in PR #8552:
URL: https://github.com/apache/hop/pull/8552#discussion_r4083710237
##########
plugins/transforms/webservices/src/main/java/org/apache/hop/pipeline/transforms/webservices/wsdl/Wsdl.java:
##########
@@ -315,65 +346,87 @@ private WSDLReader getReader() throws WSDLException {
* Load and parse the WSDL file using the wsdlLocator.
*
* @param wsdlLocator A WSDLLocator instance.
- * @param username to use for authentication
- * @param password to use for authentication
* @return wsdl Definition.
* @throws WSDLException on error.
*/
- private Definition parse(WSDLLocator wsdlLocator, String username, String password)
- throws WSDLException, HopException {
-
- WSDLReader wsdlReader = getReader();
- try {
-
- return wsdlReader.readWSDL(wsdlLocator);
- } catch (WSDLException we) {
- readWsdl(wsdlReader, wsdlURI.toString(), username, password);
- return null;
- }
+ private Definition parse(WSDLLocator wsdlLocator) throws WSDLException {
+ return getReader().readWSDL(wsdlLocator);
}
/**
- * Load and parse the WSDL file at the specified URI.
+ * Load and parse the WSDL file at the specified location.
*
- * @param wsdlURI URI of the WSDL file.
- * @param username to use for authentication
- * @param password to use for authentication
+ * @param wsdlLocation http(s) URL or Hop VFS file name of the WSDL file.
+ * @param variables to find the named VFS connections with, can be null
+ * @param username to use for HTTP authentication
+ * @param password to use for HTTP authentication
* @return wsdl Definition
* @throws WSDLException on error.
*/
- private Definition parse(URI wsdlURI, String username, String password)
+ private Definition parse(
+ String wsdlLocation, IVariables variables, String username, String password)
throws WSDLException, HopException {
+ if (StringUtils.isBlank(wsdlLocation)) {
+ throw new HopException("No WSDL location was specified");
+ }
+ String location = wsdlLocation.trim();
WSDLReader wsdlReader = getReader();
- return readWsdl(wsdlReader, wsdlURI.toString(), username, password);
+ if (isHttpLocation(location)) {
+ return readWsdl(wsdlReader, location, openHttpStream(location, username, password));
+ }
+
+ // Anything that isn't served over HTTP is a file: a plain path like the file dialog hands out,
+ // a file: URL or any other location Hop VFS knows about.
+ //
+ FileObject wsdlFile =
+ variables == null
+ ? HopVfs.getFileObject(location)
+ : HopVfs.getFileObject(location, variables);
+ try {
+ if (!wsdlFile.exists()) {
+ throw new HopException("WSDL file " + wsdlLocation + " does not exist");
+ }
+ // Imports in the WSDL are resolved relative to this URI.
+ //
+ String baseUri = wsdlFile.getName().getURI();
+ return readWsdl(wsdlReader, baseUri, HopVfs.getInputStream(wsdlFile));
Review Comment:
**[bug]** Relative `<import>` and `schemaLocation` references are not read through Hop VFS. `readWSDL(baseUri, doc)` has no `WSDLLocator`, so WSDL4J opens them with `java.net.URL` (`StringUtils.getURL` / `openStream`). A `file:` URI works, which is what `WsdlTest.resolvesImportsRelativeToTheFile` covers, but `FileName.getURI()` for `s3://`, Azure, HDFS, and similar schemes is not a protocol Java can open. On `MalformedURLException`, WSDL4J falls back to `new File(spec).toURL()`, so a WSDL that Hop VFS already read fails as soon as it has a relative import or schema include. The new manual text says those imports are looked up next to the file on any filesystem Hop can read; a self-contained WSDL still loads.
**Suggestion:** Resolve the main document and each relative location with a `WSDLLocator` that uses `HopVfs.getFileObject` against the WSDL parent (and close those streams). The same locator can attach the basic-auth header for `http(s)` imports. If that is out of scope, narrow the tooltip and the "WSDL files and imports" section to local and `file:` URLs.
##########
plugins/transforms/webservices/src/main/java/org/apache/hop/pipeline/transforms/webservices/wsdl/Wsdl.java:
##########
@@ -315,65 +346,87 @@ private WSDLReader getReader() throws WSDLException {
* Load and parse the WSDL file using the wsdlLocator.
*
* @param wsdlLocator A WSDLLocator instance.
- * @param username to use for authentication
- * @param password to use for authentication
* @return wsdl Definition.
* @throws WSDLException on error.
*/
- private Definition parse(WSDLLocator wsdlLocator, String username, String password)
- throws WSDLException, HopException {
-
- WSDLReader wsdlReader = getReader();
- try {
-
- return wsdlReader.readWSDL(wsdlLocator);
- } catch (WSDLException we) {
- readWsdl(wsdlReader, wsdlURI.toString(), username, password);
- return null;
- }
+ private Definition parse(WSDLLocator wsdlLocator) throws WSDLException {
+ return getReader().readWSDL(wsdlLocator);
}
/**
- * Load and parse the WSDL file at the specified URI.
+ * Load and parse the WSDL file at the specified location.
*
- * @param wsdlURI URI of the WSDL file.
- * @param username to use for authentication
- * @param password to use for authentication
+ * @param wsdlLocation http(s) URL or Hop VFS file name of the WSDL file.
+ * @param variables to find the named VFS connections with, can be null
+ * @param username to use for HTTP authentication
+ * @param password to use for HTTP authentication
* @return wsdl Definition
* @throws WSDLException on error.
*/
- private Definition parse(URI wsdlURI, String username, String password)
+ private Definition parse(
+ String wsdlLocation, IVariables variables, String username, String password)
throws WSDLException, HopException {
+ if (StringUtils.isBlank(wsdlLocation)) {
+ throw new HopException("No WSDL location was specified");
+ }
+ String location = wsdlLocation.trim();
WSDLReader wsdlReader = getReader();
- return readWsdl(wsdlReader, wsdlURI.toString(), username, password);
+ if (isHttpLocation(location)) {
+ return readWsdl(wsdlReader, location, openHttpStream(location, username, password));
+ }
+
+ // Anything that isn't served over HTTP is a file: a plain path like the file dialog hands out,
+ // a file: URL or any other location Hop VFS knows about.
+ //
+ FileObject wsdlFile =
Review Comment:
**[suggestion]** `wsdlFile` is never closed. `HopVfs.getInputStream` only yields a content stream, which `readWsdl` / `XmlHandler.loadXmlFile` close; the `FileObject` stays referenced by the shared VFS cache, and a remote provider can keep a connection open. The "does not exist" throw leaves the same leak.
**Suggestion:** Open the file in try-with-resources so it is closed after `readWsdl` returns, including on failure. Keep wrapping `FileSystemException` in `HopException` (it does not extend `HopException`) so the constructor still reports the location.
--
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: issues-unsubscribe@hop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org