From 0bd64838d02919d8e27c5bcad03c9f482ca8c572 Mon Sep 17 00:00:00 2001 From: 1000TurquoisePogs Date: Wed, 27 Jul 2022 17:39:27 -0400 Subject: [PATCH 1/4] Update webapp.js Signed-off-by: 1000TurquoisePogs Signed-off-by: 1000TurquoisePogs --- lib/webapp.js | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/webapp.js b/lib/webapp.js index fedfa9d4..0e95aa78 100644 --- a/lib/webapp.js +++ b/lib/webapp.js @@ -875,6 +875,7 @@ WebServiceHandle.prototype = { call(path, options, originalRequest, originalRes) { let serviceModified = true; return new Promise((resolve, reject) => { + routingLog.debug(`in 'call', path=%s`, JSON.stringify(path)); if (typeof path === "object") { options = path; path = ""; @@ -949,17 +950,21 @@ WebServiceHandle.prototype = { if(this.service && this.service.imported) { requestOptions.path = zLuxUrl.makePluginURL('ZLUX', this.service.sourcePlugin) + zLuxUrl.makeServiceSubURL(this.service, false, false, path); - routingLog.debug(`Call agent host=%s import resolved to path=%s`,requestOptions.hostname, requestOptions.path); + routingLog.debug(`Call agent host=%s port=%s import resolved to path=%s`,requestOptions.hostname, requestOptions.port, requestOptions.path); } let httpOrHttps; if (this.isAgentService || (this.service && (this.service.type === 'external' || this.service.type === 'service'))) { if(this.service && this.service.type === 'external') { + routingLog.debug(`adjusting requestOptions for service type=external`); requestOptions.hostname = this.service.host; requestOptions.port = this.service.port; - requestOptions.path = this.service.urlPrefix; + if (this.service.urlPrefix) { + requestOptions.path = this.service.urlPrefix + requestOptions.path; + } requestOptions.protocol = this.service.isHttps ? 'https:': 'http:'; } else { + routingLog.debug(`adjusting requestOptions for service type=service`); requestOptions.hostname = this.environment.agentRequestOptions.host; requestOptions.port = this.environment.agentRequestOptions.port; requestOptions.protocol = this.environment.agentRequestOptions.protocol; @@ -972,7 +977,9 @@ WebServiceHandle.prototype = { } } httpOrHttps = requestOptions.protocol == 'http:' ? http : https; - routingLog.debug(`Call agent host=%s path=%s`,requestOptions.hostname, requestOptions.path); + // prevent double slashes in url + requestOptions.path = requestOptions.path.replace(/\/\//g, '/'); + routingLog.debug(`Call external server host=%s port=%s path=%s`,requestOptions.hostname, requestOptions.port, requestOptions.path); } else { if (options.zluxLoopbackSecret) { //TODO use secret in a crypto scheme @@ -1029,7 +1036,7 @@ WebServiceHandle.prototype = { } res.end = (body)=> { - utilLog.debug('router returned with body=',body.length); + routingLog.debug('router returned with body=',body.length); res.body = iconv.decode(body,'utf8'); if (!res.statusMessage) { res.statusMessage = status[res.statusCode]; @@ -1064,24 +1071,24 @@ WebServiceHandle.prototype = { const request = httpOrHttps.request(requestOptions, (response) => { var chunks = []; response.on('data',(chunk)=> { - utilLog.debug('ZWED0194I'); //utilLog.debug('Callservice: Data received'); + routingLog.debug('ZWED0194I'); //utilLog.debug('Callservice: Data received'); chunks.push(chunk); }); response.on('end',() => { - utilLog.debug('ZWED0195I'); //utilLog.debug('Callservice: Service call completed.'); + routingLog.debug('ZWED0195I'); //utilLog.debug('Callservice: Service call completed.'); response.body = Buffer.concat(chunks).toString(); resolve(response); }); } ); request.on('error', (e) => { - utilLog.warn('ZWED0061W'); //utilLog.warn('Callservice: Service call failed.'); + routingLog.warn('ZWED0061W'); //utilLog.warn('Callservice: Service call failed.'); reject(e); }); if (options.body) { request.write(options.body); } - utilLog.debug('ZWED0196I', JSON.stringify(requestOptions, null, 2)); //utilLog.debug('Callservice: Issuing request to service: ' + routingLog.debug('ZWED0196I', JSON.stringify(requestOptions, null, 2)); //utilLog.debug('Callservice: Issuing request to service: ' //+ JSON.stringify(requestOptions, null, 2)); request.end(); } From f4fd833b714a8aa2bd5405a2d27b1055f64ff66e Mon Sep 17 00:00:00 2001 From: 1000TurquoisePogs Date: Thu, 28 Jul 2022 13:56:02 +0000 Subject: [PATCH 2/4] Update build-core.yml Signed-off-by: 1000TurquoisePogs --- .github/workflows/build-core.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-core.yml b/.github/workflows/build-core.yml index 507886a5..275040af 100644 --- a/.github/workflows/build-core.yml +++ b/.github/workflows/build-core.yml @@ -66,7 +66,7 @@ jobs: - name: '[Prep 2] Setup Node' uses: actions/setup-node@v2 with: - node-version: 16 + node-version: 14 - name: '[Prep 3] Setup jFrog CLI' uses: jfrog/setup-jfrog-cli@v2 From ece47ae58b4790970d4eee61ceac452ae1f8faf0 Mon Sep 17 00:00:00 2001 From: 1000TurquoisePogs Date: Mon, 1 Aug 2022 14:50:21 -0400 Subject: [PATCH 3/4] Fixes after second review Signed-off-by: 1000TurquoisePogs --- CHANGELOG.md | 4 ++++ lib/webapp.js | 53 +++++++++++++++++++++++++++++++-------------------- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 170a5ad4..60557d95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ All notable changes to the Zlux Server Framework package will be documented in this file. This repo is part of the app-server Zowe Component, and the change logs here may appear on Zowe.org in that section. +## 1.28.1 + +- Bugfix: call() on an external service would be missing the full URL, causing invalid response or hang + ## 1.28.0 - Bugfix: Pass through tlsOptions object when making a proxy from an 'external'-type service, and allow the services to individually control tls verification strictness of their own proxy. diff --git a/lib/webapp.js b/lib/webapp.js index 0e95aa78..46e4c4d1 100644 --- a/lib/webapp.js +++ b/lib/webapp.js @@ -851,8 +851,8 @@ const staticHandlers = { * This is passed to every other service of the plugin, so that * the service can be called by other services under the plugin */ -function WebServiceHandle(urlPrefix, environment, isAgentService, service) { - this.urlPrefix = urlPrefix; +function WebServiceHandle(serviceRootUrl, environment, isAgentService, service) { + this.serviceRootUrl = serviceRootUrl; if (!environment.loopbackConfig.port) { installLog.severe(`ZWED0003E`, loopbackConfig); //installLog.severe(`loopback configuration not valid,`,loopbackConfig, `loopback calls will fail!`); } @@ -862,29 +862,35 @@ function WebServiceHandle(urlPrefix, environment, isAgentService, service) { } WebServiceHandle.prototype = { constructor: WebServiceHandle, - //This is currently suboptimal: it makes an HTTP call - //to localhost for every service call. We could instead just call - //the corresponding router directly with mock request and - //response objects, but that's tricky, so let's do that - //later. - // router: null, port: 0, - urlPrefix: null, + + // The route on this server to the service's root, such as + // /ZLUX/plugins/org.zowe.zlux.sample.angular/services/hello/1.0.1 + serviceRootUrl: null, + + /* + path: A path url on the destiation. Can be prefixed with the route to the service root conditional to service type. + options: An object containing options to modify the request behavior + originalRequest: the request object from the source service that issued call(). don't modify it!! + originalRes: the response object from the source service that issued call(). don't modify it!! + */ call(path, options, originalRequest, originalRes) { let serviceModified = true; return new Promise((resolve, reject) => { - routingLog.debug(`in 'call', path=%s`, JSON.stringify(path)); if (typeof path === "object") { options = path; path = ""; } - options = options || {}; - let url = this.urlPrefix; - if (path) { - url += path.startsWith('/') ? path : '/' + path; + //can be changed if service is an import, will be resolved to source service + let rootUrl = this.serviceRootUrl; + if (typeof path === 'string' && path.length != 0 && !path.startsWith('/')) { + path = '/' + path; //ensure separation between serviceRootUrl and path } + routingLog.debug(`in 'call()', path=%s`, path); + + options = options || {}; let rejectUnauthorized; let protocol; if (this.environment.loopbackConfig.isHttps) { @@ -898,7 +904,6 @@ WebServiceHandle.prototype = { port: this.environment.loopbackConfig.port, method: options.method || "GET", protocol: protocol, - path: url, auth: options.auth, timeout: options.timeout, rejectUnauthorized: rejectUnauthorized @@ -949,7 +954,8 @@ WebServiceHandle.prototype = { } if(this.service && this.service.imported) { - requestOptions.path = zLuxUrl.makePluginURL('ZLUX', this.service.sourcePlugin) + zLuxUrl.makeServiceSubURL(this.service, false, false, path); + //reassign rootUrl which will be consumed in lines below for full url + rootUrl = zLuxUrl.makePluginURL('ZLUX', this.service.sourcePlugin) + zLuxUrl.makeServiceSubURL(this.service, false, false, undefined); routingLog.debug(`Call agent host=%s port=%s import resolved to path=%s`,requestOptions.hostname, requestOptions.port, requestOptions.path); } @@ -959,15 +965,15 @@ WebServiceHandle.prototype = { routingLog.debug(`adjusting requestOptions for service type=external`); requestOptions.hostname = this.service.host; requestOptions.port = this.service.port; - if (this.service.urlPrefix) { - requestOptions.path = this.service.urlPrefix + requestOptions.path; - } requestOptions.protocol = this.service.isHttps ? 'https:': 'http:'; + requestOptions.path = this.service.urlPrefix + ? this.service.urlPrefix + path : path; } else { routingLog.debug(`adjusting requestOptions for service type=service`); requestOptions.hostname = this.environment.agentRequestOptions.host; requestOptions.port = this.environment.agentRequestOptions.port; requestOptions.protocol = this.environment.agentRequestOptions.protocol; + requestOptions.path = path ? rootUrl + path : rootUrl; requestOptions.rejectUnauthorized = this.environment.agentRequestOptions.rejectUnauthorized; requestOptions.ca = this.environment.agentRequestOptions.ca; requestOptions.ciphers = this.environment.agentRequestOptions.ciphers; @@ -979,15 +985,20 @@ WebServiceHandle.prototype = { httpOrHttps = requestOptions.protocol == 'http:' ? http : https; // prevent double slashes in url requestOptions.path = requestOptions.path.replace(/\/\//g, '/'); - routingLog.debug(`Call external server host=%s port=%s path=%s`,requestOptions.hostname, requestOptions.port, requestOptions.path); + routingLog.debug(`Call external server host=%s port=%s requestOptions.path=%s`,requestOptions.hostname, requestOptions.port, requestOptions.path); } else { + requestOptions.path = path ? rootUrl + path : rootUrl; + // prevent double slashes in url + requestOptions.path = requestOptions.path.replace(/\/\//g, '/'); + + if (options.zluxLoopbackSecret) { //TODO use secret in a crypto scheme requestOptions.headers[ZLUX_LOOPBACK_HEADER] = options.zluxLoopbackSecret; } if (options.internalRouting) { - routingLog.debug(`Call internally path=%s`, requestOptions.path); + routingLog.debug(`Call internally requestOptions.path=%s`, requestOptions.path); //clone done to prevent original request alteration, but shallow so hope attributes arent altered let req = Object.assign({},originalRequest); let res = Object.assign({},originalRes); From a07527c4dfd58de345b04524526d93fa1bec2fe2 Mon Sep 17 00:00:00 2001 From: 1000TurquoisePogs Date: Mon, 1 Aug 2022 16:01:28 -0400 Subject: [PATCH 4/4] Sanitize path input ore Signed-off-by: 1000TurquoisePogs --- lib/webapp.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/webapp.js b/lib/webapp.js index 46e4c4d1..09d1fbfc 100644 --- a/lib/webapp.js +++ b/lib/webapp.js @@ -885,9 +885,13 @@ WebServiceHandle.prototype = { } //can be changed if service is an import, will be resolved to source service let rootUrl = this.serviceRootUrl; - if (typeof path === 'string' && path.length != 0 && !path.startsWith('/')) { + if (typeof path != 'string') { + path=""; + } + if (path.length != 0 && !path.startsWith('/')) { path = '/' + path; //ensure separation between serviceRootUrl and path } + //at this point path is a string that is either blank or starts with / routingLog.debug(`in 'call()', path=%s`, path); options = options || {}; @@ -966,14 +970,14 @@ WebServiceHandle.prototype = { requestOptions.hostname = this.service.host; requestOptions.port = this.service.port; requestOptions.protocol = this.service.isHttps ? 'https:': 'http:'; - requestOptions.path = this.service.urlPrefix - ? this.service.urlPrefix + path : path; + requestOptions.path = this.service.urlPrefix && this.service.urlPrefix.length > 0 + ? this.service.urlPrefix + path : path; //could be empty string } else { routingLog.debug(`adjusting requestOptions for service type=service`); requestOptions.hostname = this.environment.agentRequestOptions.host; requestOptions.port = this.environment.agentRequestOptions.port; requestOptions.protocol = this.environment.agentRequestOptions.protocol; - requestOptions.path = path ? rootUrl + path : rootUrl; + requestOptions.path = rootUrl + path; //path may be empty string and thats ok requestOptions.rejectUnauthorized = this.environment.agentRequestOptions.rejectUnauthorized; requestOptions.ca = this.environment.agentRequestOptions.ca; requestOptions.ciphers = this.environment.agentRequestOptions.ciphers; @@ -983,7 +987,7 @@ WebServiceHandle.prototype = { } } httpOrHttps = requestOptions.protocol == 'http:' ? http : https; - // prevent double slashes in url + // prevent double slashes in url, if you have more than double slash then good luck requestOptions.path = requestOptions.path.replace(/\/\//g, '/'); routingLog.debug(`Call external server host=%s port=%s requestOptions.path=%s`,requestOptions.hostname, requestOptions.port, requestOptions.path); } else {