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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
