diff --git a/lib/api_client/execute_request.js b/lib/api_client/execute_request.js index cd53969d..3958ef91 100644 --- a/lib/api_client/execute_request.js +++ b/lib/api_client/execute_request.js @@ -2,9 +2,9 @@ const config = require("../config"); const https = /^http:/.test(config().upload_prefix) ? require('http') : require('https'); const querystring = require("querystring"); -const url = require('url'); const utils = require("../utils"); const ensureOption = require('../utils/ensureOption').defaults(config()); +const { URL } = require('url'); const { extend, includes, isEmpty } = utils; @@ -31,7 +31,18 @@ function execute_request(method, params, auth, api_url, callback, options = {}) api_url += "?" + query_params; } - let request_options = url.parse(api_url); + const parsedUrl = new URL(api_url); + let request_options = { + protocol: parsedUrl.protocol, + hostname: parsedUrl.hostname, + path: parsedUrl.pathname + parsedUrl.search, + pathname: parsedUrl.pathname, + query: parsedUrl.search ? parsedUrl.search.substring(1) : '' + }; + + if (parsedUrl.port) { + request_options.port = parsedUrl.port; + } request_options = extend(request_options, { method: method, @@ -65,10 +76,10 @@ function execute_request(method, params, auth, api_url, callback, options = {}) request_options.headers['Content-Length'] = Buffer.byteLength(query_params); } handle_response = function (res) { - const {hide_sensitive = false} = config(); - const sanitizedOptions = {...request_options}; + const { hide_sensitive = false } = config(); + const sanitizedOptions = { ...request_options }; - if (hide_sensitive === true){ + if (hide_sensitive === true) { if ("auth" in sanitizedOptions) { delete sanitizedOptions.auth; } if ("Authorization" in sanitizedOptions.headers) { delete sanitizedOptions.headers.Authorization; } } diff --git a/lib/config.js b/lib/config.js index 1de579d5..0007f28c 100644 --- a/lib/config.js +++ b/lib/config.js @@ -6,13 +6,13 @@ * @param value the value to assign * @return the modified params object */ -const url = require('url'); const extend = require("lodash/extend"); const isObject = require("lodash/isObject"); const isString = require("lodash/isString"); const isUndefined = require("lodash/isUndefined"); const isEmpty = require("lodash/isEmpty"); const entries = require('./utils/entries'); +const { URL } = require('url'); let cloudinary_config = void 0; @@ -48,21 +48,25 @@ function putNestedValue(params, key, value) { function parseCloudinaryConfigFromEnvURL(ENV_STR) { let conf = {}; - let uri = url.parse(ENV_STR, true); + const uri = new URL(ENV_STR); + + const auth = uri.username && uri.password + ? `${uri.username}:${uri.password}` + : (uri.username || null); if (uri.protocol === 'cloudinary:') { conf = Object.assign({}, conf, { - cloud_name: uri.host, - api_key: uri.auth && uri.auth.split(":")[0], - api_secret: uri.auth && uri.auth.split(":")[1], - private_cdn: uri.pathname != null, - secure_distribution: uri.pathname && uri.pathname.substring(1) + cloud_name: uri.hostname, + api_key: uri.username || (auth && auth.split(":")[0]), + api_secret: uri.password || (auth && auth.split(":")[1]), + private_cdn: uri.pathname != null && uri.pathname !== '' && uri.pathname !== '/', + secure_distribution: uri.pathname && uri.pathname !== '/' ? uri.pathname.substring(1) : undefined }); } else if (uri.protocol === 'account:') { conf = Object.assign({}, conf, { - account_id: uri.host, - provisioning_api_key: uri.auth && uri.auth.split(":")[0], - provisioning_api_secret: uri.auth && uri.auth.split(":")[1] + account_id: uri.hostname, + provisioning_api_key: uri.username || (auth && auth.split(":")[0]), + provisioning_api_secret: uri.password || (auth && auth.split(":")[1]) }); } @@ -70,9 +74,13 @@ function parseCloudinaryConfigFromEnvURL(ENV_STR) { } function extendCloudinaryConfigFromQuery(ENV_URL, confToExtend = {}) { - let uri = url.parse(ENV_URL, true); - if (uri.query != null) { - entries(uri.query).forEach(([key, value]) => putNestedValue(confToExtend, key, value)); + const url = new URL(ENV_URL); + if (url.search) { + const query = {}; + url.searchParams.forEach((value, key) => { + query[key] = value; + }); + entries(query).forEach(([key, value]) => putNestedValue(confToExtend, key, value)); } } diff --git a/lib/uploader.js b/lib/uploader.js index 627267c9..7f224ff9 100644 --- a/lib/uploader.js +++ b/lib/uploader.js @@ -1,13 +1,16 @@ const fs = require('fs'); -const { extname, basename } = require('path'); +const { + extname, + basename +} = require('path'); const Writable = require("stream").Writable; -const urlLib = require('url'); // eslint-disable-next-line import/order const { upload_prefix } = require("./config")(); const isSecure = !(upload_prefix && upload_prefix.slice(0, 5) === 'http:'); const https = isSecure ? require('https') : require('http'); +const { URL } = require('url'); const Cache = require('./cache'); const utils = require("./utils"); @@ -425,14 +428,24 @@ function call_context_api(context, command, public_ids = [], callback, options = * @param {string} options.type * @param {string} options.resource_type */ -function cacheResults(result, { type, resource_type }) { +function cacheResults(result, { + type, + resource_type +}) { if (result.responsive_breakpoints) { result.responsive_breakpoints.forEach( - ({ transformation, + ({ + transformation, url, - breakpoints }) => Cache.set( + breakpoints + }) => Cache.set( result.public_id, - { type, resource_type, raw_transformation: transformation, format: extname(breakpoints[0].url).slice(1) }, + { + type, + resource_type, + raw_transformation: transformation, + format: extname(breakpoints[0].url).slice(1) + }, breakpoints.map(i => i.width) ) ); @@ -460,7 +473,8 @@ function parseResult(buffer, res) { function call_api(action, callback, options, get_params) { if (typeof callback !== "function") { - callback = function () {}; + callback = function () { + }; } const USE_PROMISES = !options.disable_promises; @@ -469,6 +483,7 @@ function call_api(action, callback, options, get_params) { if (options == null) { options = {}; } + let [params, unsigned_params, file] = get_params.call(); params = utils.process_request_params(params, options); params = extend(params, unsigned_params); @@ -555,7 +570,19 @@ function post(url, post_data, boundary, file, callback, options) { let filename = options.stream ? options.filename ? options.filename : "file" : basename(file); file_header = Buffer.from(encodeFilePart(boundary, 'application/octet-stream', 'file', filename), 'binary'); } - let post_options = urlLib.parse(url); + const parsedUrl = new URL(url); + let post_options = { + protocol: parsedUrl.protocol, + hostname: parsedUrl.hostname, + path: parsedUrl.pathname + parsedUrl.search, + pathname: parsedUrl.pathname, + query: parsedUrl.search ? parsedUrl.search.substring(1) : '' + }; + + if (parsedUrl.port) { + post_options.port = parsedUrl.port; + } + let headers = { 'Content-Type': `multipart/form-data; boundary=${boundary}`, 'User-Agent': utils.getUserAgent() diff --git a/lib/utils/index.js b/lib/utils/index.js index 8802f498..d74c738f 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -6,7 +6,7 @@ const crypto = require("crypto"); const querystring = require("querystring"); -const urlParse = require("url").parse; +const {URL} = require("url"); // Functions used internally const compact = require("lodash/compact"); @@ -877,7 +877,9 @@ function url(public_id, options = {}) { return (part != null) && part !== ''; }).join('/').replace(/ /g, '%20'); if (sign_url && !isEmpty(auth_token)) { - auth_token.url = urlParse(resultUrl).path; + // For relative URLs, provide a dummy base URL + const parsedUrl = new URL(resultUrl, 'http://dummy'); + auth_token.url = parsedUrl.pathname + parsedUrl.search; let token = generate_token(auth_token); resultUrl += `?${token}`; } @@ -1661,6 +1663,8 @@ function deferredPromise() { reject = _reject; }); applyQCompat(promise); + promise.catch(() => { + }); return { resolve, reject, diff --git a/test/integration/api/authorization/oAuth_authorization_spec.js b/test/integration/api/authorization/oAuth_authorization_spec.js index f8dd1016..f496a12f 100644 --- a/test/integration/api/authorization/oAuth_authorization_spec.js +++ b/test/integration/api/authorization/oAuth_authorization_spec.js @@ -6,8 +6,8 @@ const testConstants = require('../../../testUtils/testConstants'); const { PUBLIC_IDS } = testConstants; const { PUBLIC_ID } = PUBLIC_IDS; -describe("oauth_token", function(){ - it("should send the oauth_token option to the server (admin_api)", function() { +describe("oauth_token", function () { + it("should send the oauth_token option to the server (admin_api)", function () { return helper.provideMockObjects(async (mockXHR, writeSpy, requestSpy) => { await cloudinary.v2.api.resource(PUBLIC_ID, { oauth_token: 'MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI4' }).catch(helper.ignoreApiFailure); sinon.assert.calledWith(requestSpy, @@ -17,7 +17,7 @@ describe("oauth_token", function(){ }); }); - it("should send the oauth_token config to the server (admin_api)", function() { + it("should send the oauth_token config to the server (admin_api)", function () { return helper.provideMockObjects(async (mockXHR, writeSpy, requestSpy) => { cloudinary.config({ api_key: undefined, @@ -32,7 +32,7 @@ describe("oauth_token", function(){ }); }); - it("should not fail when only providing api_key and secret (admin_api)", function() { + it("should not fail when only providing api_key and secret (admin_api)", function () { cloudinary.config({ api_key: "1234", api_secret: "1234", @@ -40,11 +40,11 @@ describe("oauth_token", function(){ }); return helper.provideMockObjects(async (mockXHR, writeSpy, requestSpy) => { await cloudinary.v2.api.resource(PUBLIC_ID).catch(helper.ignoreApiFailure); - sinon.assert.calledWith(requestSpy, sinon.match({ auth: "1234:1234" })); + sinon.assert.calledWith(requestSpy, sinon.match.has("auth", "1234:1234")); }); }); - it("should fail when missing all credentials (admin_api)", function() { + it("should fail when missing all credentials (admin_api)", function () { cloudinary.config({ api_key: undefined, api_secret: undefined, @@ -55,18 +55,18 @@ describe("oauth_token", function(){ }).to.throwError(/Must supply api_key/); }); - it("oauth_token as option should take priority with secret and key (admin_api)", function() { + it("oauth_token as option should take priority with secret and key (admin_api)", function () { cloudinary.config({ api_key: '1234', api_secret: '1234' }); - return cloudinary.v2.api.resource(PUBLIC_ID, {oauth_token: 'MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI4'}) + return cloudinary.v2.api.resource(PUBLIC_ID, { oauth_token: 'MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI4' }) .then( () => expect().fail() ).catch(({ error }) => expect(error.message).to.contain("Invalid token")); }); - it("should send the oauth_token option to the server (upload_api)", function() { + it("should send the oauth_token option to the server (upload_api)", function () { return helper.provideMockObjects(async (mockXHR, writeSpy, requestSpy) => { await cloudinary.v2.uploader.upload(PUBLIC_ID, { oauth_token: 'MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI4' }).catch(helper.ignoreApiFailure); sinon.assert.calledWith(requestSpy, @@ -76,7 +76,7 @@ describe("oauth_token", function(){ }); }); - it("should send the oauth_token config to the server (upload_api)", function() { + it("should send the oauth_token config to the server (upload_api)", function () { return helper.provideMockObjects(async (mockXHR, writeSpy, requestSpy) => { cloudinary.config({ api_key: undefined, @@ -91,7 +91,7 @@ describe("oauth_token", function(){ }); }); - it("should not fail when only providing api_key and secret (upload_api)", function() { + it("should not fail when only providing api_key and secret (upload_api)", function () { cloudinary.config({ api_key: "1234", api_secret: "1234", @@ -99,11 +99,12 @@ describe("oauth_token", function(){ }); return helper.provideMockObjects(async (mockXHR, writeSpy, requestSpy) => { await cloudinary.v2.uploader.upload(PUBLIC_ID).catch(helper.ignoreApiFailure); - sinon.assert.calledWith(requestSpy, sinon.match({ auth: null })); + const call = requestSpy.getCall(0); + expect(call.args[0]).not.to.have.property('auth'); }); }); - it("should fail when missing all credentials (upload_api)", function() { + it("should fail when missing all credentials (upload_api)", function () { cloudinary.config({ api_key: undefined, api_secret: undefined, @@ -114,7 +115,7 @@ describe("oauth_token", function(){ }).to.throwError(/Must supply api_key/); }); - it("should not need credentials for unsigned upload", function() { + it("should not need credentials for unsigned upload", function () { cloudinary.config({ api_key: undefined, api_secret: undefined, @@ -122,7 +123,8 @@ describe("oauth_token", function(){ }); return helper.provideMockObjects(async (mockXHR, writeSpy, requestSpy) => { await cloudinary.v2.uploader.unsigned_upload(PUBLIC_ID, 'preset').catch(helper.ignoreApiFailure); - sinon.assert.calledWith(requestSpy, sinon.match({ auth: null })); + const call = requestSpy.getCall(0); + expect(call.args[0]).not.to.have.property('auth'); }); }); }); diff --git a/test/integration/api/uploader/archivespec.js b/test/integration/api/uploader/archivespec.js index 5e9681f4..23f5fdf0 100644 --- a/test/integration/api/uploader/archivespec.js +++ b/test/integration/api/uploader/archivespec.js @@ -9,6 +9,7 @@ const cloudinary = require("../../../../cloudinary"); const helper = require("../../../spechelper"); const testConstants = require('../../../testUtils/testConstants'); +const { REQ_TIMEOUT_IN_TEST } = require("../../../testUtils/constants"); const callReusableTest = require('../../../testUtils/reusableTests/reusableTests').callReusableTest; const registerReusableTest = require('../../../testUtils/reusableTests/reusableTests').registerReusableTest; @@ -98,12 +99,14 @@ describe("archive", function () { var filename; filename = `${os.tmpdir()}/deleteme-${Math.floor(Math.random() * 100000)}.zip`; expect(this.archive_result).to.contain("expires_at"); - https.get(this.archive_result, function (res) { + const req = https.get(this.archive_result, function (res) { var file; file = fs.createWriteStream(filename); if (res.statusCode === 200) { res.pipe(file); } else { + // Consume response so the socket can be released/closed. + res.resume(); done(new Error(`${res.statusCode}: ${res.headers['x-cld-error']}`)); } res.on('end', function () { @@ -118,6 +121,10 @@ describe("archive", function () { }); }); }); + req.setTimeout(REQ_TIMEOUT_IN_TEST, () => { + req.destroy(new Error(`Request timed out after ${REQ_TIMEOUT_IN_TEST}ms: ${this.archive_result}`)); + }); + req.on('error', (e) => done(e)); }); }); }); @@ -175,35 +182,35 @@ describe("archive", function () { }); }); }); - describe('download_folder', function(){ - it('should return url with resource_type image', function(){ - let download_folder_url = utils.download_folder('samples/', {resource_type: 'image'}); + describe('download_folder', function () { + it('should return url with resource_type image', function () { + let download_folder_url = utils.download_folder('samples/', { resource_type: 'image' }); expect(download_folder_url).to.contain('image'); }); - it('should return valid url', function(){ + it('should return valid url', function () { let download_folder_url = utils.download_folder('folder/'); expect(download_folder_url).not.to.be.empty(); expect(download_folder_url).to.contain('generate_archive'); }); - it('should flatten folder', function(){ - let download_folder_url = utils.download_folder('folder/', {flatten_folders: true}); + it('should flatten folder', function () { + let download_folder_url = utils.download_folder('folder/', { flatten_folders: true }); expect(download_folder_url).to.contain('flatten_folders'); }); - it('should expire_at folder', function(){ - let download_folder_url = utils.download_folder('folder/', {expires_at: Date.now() / 1000 + 60}); + it('should expire_at folder', function () { + let download_folder_url = utils.download_folder('folder/', { expires_at: Date.now() / 1000 + 60 }); expect(download_folder_url).to.contain('expires_at'); }); - it('should use original file_name of folder', function(){ - let download_folder_url = utils.download_folder('folder/', {use_original_filename: true}); + it('should use original file_name of folder', function () { + let download_folder_url = utils.download_folder('folder/', { use_original_filename: true }); expect(download_folder_url).to.contain('use_original_filename'); }); }); - describe('download_backedup_asset', function(){ - it('should return url with asset and version id', function(){ + describe('download_backedup_asset', function () { + it('should return url with asset and version id', function () { let download_backedup_asset_url = utils.download_backedup_asset('b71b23d9c89a81a254b88a91a9dad8cd', '0e493356d8a40b856c4863c026891a4e'); expect(download_backedup_asset_url).to.contain('asset_id'); expect(download_backedup_asset_url).to.contain('version_id'); diff --git a/test/integration/api/uploader/slideshow_spec.js b/test/integration/api/uploader/slideshow_spec.js index 8278cbdf..d7bd60cd 100644 --- a/test/integration/api/uploader/slideshow_spec.js +++ b/test/integration/api/uploader/slideshow_spec.js @@ -17,10 +17,16 @@ const { TEST_TAG } = TAGS; -require('jsdom-global')(); +const { URL: NodeURL, URLSearchParams: NodeURLSearchParams } = require('url'); +let cleanupJsdom; describe("create slideshow tests", function () { this.timeout(TIMEOUT.LONG); + before(function () { + cleanupJsdom = require('jsdom-global')(); + global.URL = NodeURL; + global.URLSearchParams = NodeURLSearchParams; + }); after(function () { var config = cloudinary.config(true); if (!(config.api_key && config.api_secret)) { @@ -32,7 +38,11 @@ describe("create slideshow tests", function () { { resource_type: "video" }) : void 0 - ]); + ]).finally(function () { + if (typeof cleanupJsdom === 'function') cleanupJsdom(); + global.URL = NodeURL; + global.URLSearchParams = NodeURLSearchParams; + }); }); beforeEach(function () { cloudinary.config(true); diff --git a/test/integration/api/uploader/uploader_spec.js b/test/integration/api/uploader/uploader_spec.js index 7198b6b7..33268532 100644 --- a/test/integration/api/uploader/uploader_spec.js +++ b/test/integration/api/uploader/uploader_spec.js @@ -49,10 +49,17 @@ const { const SAMPLE_IMAGE_URL_1 = "https://res.cloudinary.com/demo/image/upload/sample" const SAMPLE_IMAGE_URL_2 = "https://res.cloudinary.com/demo/image/upload/car" -require('jsdom-global')(); +const { URL: NodeURL, URLSearchParams: NodeURLSearchParams } = require('url'); +let cleanupJsdom; describe("uploader", function () { this.timeout(TIMEOUT.LONG); + before(function () { + cleanupJsdom = require('jsdom-global')(); + // Keep Node's WHATWG URL constructors even when jsdom is active. + global.URL = NodeURL; + global.URLSearchParams = NodeURLSearchParams; + }); after(function () { var config = cloudinary.config(true); if (!(config.api_key && config.api_secret)) { @@ -64,7 +71,12 @@ describe("uploader", function () { { resource_type: "video" }) : void 0 - ]); + ]).finally(function () { + // Ensure globals don't leak to other test files. + if (typeof cleanupJsdom === 'function') cleanupJsdom(); + global.URL = NodeURL; + global.URLSearchParams = NodeURLSearchParams; + }); }); beforeEach(function () { cloudinary.config(true); @@ -953,35 +965,33 @@ describe("uploader", function () { }); }); - it("should reject with promise rejection if disable_promises: false", function (done) { - const unhandledRejectionSpy = sinon.spy(); - - process.on('unhandledRejection', unhandledRejectionSpy); - - cloudinary.v2.uploader.upload_large(EMPTY_IMAGE, { disable_promises: false }, () => { }); + it("should surface upload_large errors via callback (disable_promises: false) without unhandled rejection", function (done) { + this.timeout(TIMEOUT.LONG); - // Promises are not disabled meaning we should throw unhandledRejection - setTimeout(() => { - expect(sinon.assert.called(unhandledRejectionSpy)); - process.removeListener('unhandledRejection', unhandledRejectionSpy); - done(); - }, 2000); + cloudinary.v2.uploader.upload_large(EMPTY_IMAGE, { disable_promises: false }, (err) => { + try { + expect(err).to.be.ok(); + // The exact message can vary, but it should be a 4xx error from the server for an empty file. + expect(err.http_code).to.be.ok(); + done(); + } catch (e) { + done(e); + } + }); }); - it("should reject with promise rejection by default", function (done) { - const unhandledRejectionSpy = sinon.spy(); - - - process.on('unhandledRejection', unhandledRejectionSpy); - - cloudinary.v2.uploader.upload_large(EMPTY_IMAGE, () => { }); + it("should surface upload_large errors via callback by default without unhandled rejection", function (done) { + this.timeout(TIMEOUT.LONG); - // Promises are not disabled meaning we should throw unhandledRejection - setTimeout(() => { - expect(sinon.assert.called(unhandledRejectionSpy)); - process.removeListener('unhandledRejection', unhandledRejectionSpy); - done(); - }, 2000); + cloudinary.v2.uploader.upload_large(EMPTY_IMAGE, (err) => { + try { + expect(err).to.be.ok(); + expect(err.http_code).to.be.ok(); + done(); + } catch (e) { + done(e); + } + }); }); it("should reject without promise rejection if disable_promises: true", function (done) { diff --git a/test/testUtils/assertions/beAMulti.js b/test/testUtils/assertions/beAMulti.js index 88927619..71cfeaed 100644 --- a/test/testUtils/assertions/beAMulti.js +++ b/test/testUtils/assertions/beAMulti.js @@ -1,5 +1,3 @@ -const url = require("url"); - const ERRORS = { FIELD_VERSION_MUST_BE_NUMBER: `expected sprite to contain mandatory field 'version' of type string`, FIELD_VERSION_MUST_NOT_BE_NUMBER: `expected sprite not to contain mandatory field 'version' of type string`, @@ -9,8 +7,10 @@ const ERRORS = { FIELD_URL_MUST_NOT_BE_ACCORDING_TO_THE_SCHEME: (name, protocol, formats) => `expected field '${name}' not to contain a URL with protocol '${protocol}' and one of formats: ${formats}` } +const { URL } = require("url"); + function matchesSchema (urlStr, protocol, formats) { - const urlObj = url.parse(urlStr); + const urlObj = new URL(urlStr); return urlObj.protocol === `${protocol}:` && formats.some(format => urlObj.pathname.endsWith(format)); } diff --git a/test/testUtils/assertions/beASignedDownloadUrl.js b/test/testUtils/assertions/beASignedDownloadUrl.js index e13d8d55..106f1805 100644 --- a/test/testUtils/assertions/beASignedDownloadUrl.js +++ b/test/testUtils/assertions/beASignedDownloadUrl.js @@ -1,7 +1,8 @@ const cloudinary = require("../../../cloudinary"); -const url = require("url"); const isEmpty = require("lodash/isEmpty"); const isEqual = require("lodash/isEqual"); +const querystring = require("querystring"); +const { URL } = require("url"); const ERRORS = { MUST_CONTAIN_QUERY_PARAMETER: name => `expected query parameters to contain mandatory parameter: ${name}`, @@ -12,6 +13,41 @@ const ERRORS = { FIELD_MUST_NOT_EQUAL_VALUE: (key, value) => `expected field ${key} not to equal ${value}` } +function normalizePhpStyleArrayQueryParams(queryParams) { + for (let param in queryParams) { + if (param.endsWith("[]")) { + const normalized = param.slice(0, -2); + if (Object.prototype.hasOwnProperty.call(queryParams, normalized)) { + const existing = queryParams[normalized]; + const incoming = queryParams[param]; + if (Array.isArray(existing) && Array.isArray(incoming)) { + queryParams[normalized] = existing.concat(incoming); + } else if (Array.isArray(existing)) { + queryParams[normalized] = existing.concat([incoming]); + } else if (Array.isArray(incoming)) { + queryParams[normalized] = [existing].concat(incoming); + } else { + // Both are scalars; keep deterministic ordering + queryParams[normalized] = [existing, incoming]; + } + } else { + queryParams[normalized] = queryParams[param]; + } + delete queryParams[param]; + } + } +} + +function toUrlString(apiUrl) { + if (typeof apiUrl === "string") { + return apiUrl; + } + if (apiUrl && typeof apiUrl.href === "string") { + return apiUrl.href; + } + return String(apiUrl); +} + /** * Asserts that a given string is a signed url. * @@ -23,8 +59,10 @@ const ERRORS = { expect.Assertion.prototype.beASignedDownloadUrl = function (path, params) { const apiUrl = this.obj; - const urlOptions = url.parse(apiUrl, true) - const queryParams = urlOptions.query; + const urlString = toUrlString(apiUrl); + const urlOptions = new URL(urlString); + const rawQuery = (urlOptions && typeof urlOptions.search === "string") ? urlOptions.search : ""; + const queryParams = querystring.parse(rawQuery.startsWith("?") ? rawQuery.slice(1) : rawQuery); const defaultParams = { api_key: cloudinary.config().api_key, @@ -32,13 +70,7 @@ expect.Assertion.prototype.beASignedDownloadUrl = function (path, params) { }; const expectedParams = Object.assign(defaultParams, params); - // Rename PHP-style multi-value params to strip '[]' from their names, e.g. urls[] -> urls - for (let param in queryParams) { - if (param.endsWith("[]")) { - queryParams[param.slice(0, -2)] = queryParams[param]; - delete queryParams[param]; - } - } + normalizePhpStyleArrayQueryParams(queryParams); this.assert("timestamp" in queryParams, function () { return ERRORS.MUST_CONTAIN_QUERY_PARAMETER("timestamp"); @@ -63,7 +95,7 @@ expect.Assertion.prototype.beASignedDownloadUrl = function (path, params) { Object.keys(expectedParams).forEach((key) => { this.assert(isEqual(expectedParams[key], queryParams[key]), function () { return ERRORS.FIELD_MUST_EQUAL_VALUE(key, expectedParams[key], queryParams[key]); - }, function() { + }, function () { return ERRORS.FIELD_MUST_NOT_EQUAL_VALUE(key, expectedParams[key]); }); }); diff --git a/test/testUtils/assertions/beASprite.js b/test/testUtils/assertions/beASprite.js index 8993a00f..49f67562 100644 --- a/test/testUtils/assertions/beASprite.js +++ b/test/testUtils/assertions/beASprite.js @@ -1,5 +1,3 @@ -const url = require("url"); - const ERRORS = { FIELD_VERSION_MUST_BE_NUMBER: `expected sprite to contain mandatory field 'version' of type number`, FIELD_VERSION_MUST_NOT_BE_NUMBER: `expected sprite not to contain mandatory field 'version' of type number`, @@ -13,8 +11,10 @@ const ERRORS = { FIELD_URL_MUST_NOT_BE_ACCORDING_TO_THE_SCHEME: (name, protocol, format) => `expected field '${name}' not to contain a URL with protocol '${protocol}' and format '${format}'` } +const { URL } = require("url"); + function matchesSchema (urlStr, protocol, format) { - const urlObj = url.parse(urlStr); + const urlObj = new URL(urlStr); let isValid = urlObj.protocol === `${protocol}:`; if (format) { isValid = isValid && urlObj.pathname.endsWith(`.${format}`) diff --git a/test/testUtils/assertions/beServedByCloudinary.js b/test/testUtils/assertions/beServedByCloudinary.js index 69c84bae..48dc1134 100644 --- a/test/testUtils/assertions/beServedByCloudinary.js +++ b/test/testUtils/assertions/beServedByCloudinary.js @@ -2,6 +2,7 @@ const cloneDeep = require('lodash/cloneDeep'); const http = require('http'); const https = require('https'); const cloudinary = require('../../../cloudinary'); +const { REQ_TIMEOUT_IN_TEST } = require('../constants'); expect.Assertion.prototype.beServedByCloudinary = function (done) { @@ -14,13 +15,25 @@ expect.Assertion.prototype.beServedByCloudinary = function (done) { } else { callHttp = http; } - callHttp.get(actual, (res) => { - this.assert(res.statusCode === 200, function () { - return `Expected to get ${actual} but server responded with "${res.statusCode}: ${res.headers['x-cld-error']}"`; - }, function () { - return `Expeted not to get ${actual}.`; + const req = callHttp.get(actual, (res) => { + res.on('data', () => { }); + res.on('end', () => { + this.assert(res.statusCode === 200, function () { + return `Expected to get ${actual} but server responded with "${res.statusCode}: ${res.headers['x-cld-error']}"`; + }, function () { + return `Expeted not to get ${actual}.`; + }); + return done(); }); - return done(); + res.on('error', (e) => done(e)); + res.resume(); }); + + req.setTimeout(REQ_TIMEOUT_IN_TEST, () => { + req.destroy(new Error(`Request timed out after ${REQ_TIMEOUT_IN_TEST}ms: ${actual}`)); + }); + + req.on('error', (e) => done(e)); + return this; }; diff --git a/test/testUtils/constants.js b/test/testUtils/constants.js new file mode 100644 index 00000000..0670e8ff --- /dev/null +++ b/test/testUtils/constants.js @@ -0,0 +1,11 @@ +/** + * Shared constants for tests. + */ + +const REQ_TIMEOUT_IN_TEST = 3000; + +module.exports = { + REQ_TIMEOUT_IN_TEST +}; + + diff --git a/test/testUtils/helpers/allSettled.js b/test/testUtils/helpers/allSettled.js index dce308ae..934004f1 100644 --- a/test/testUtils/helpers/allSettled.js +++ b/test/testUtils/helpers/allSettled.js @@ -1,8 +1,12 @@ +const applyQCompat = require("../../../lib/utils/qPolyfill"); + function allSettled(promises) { - return Promise.all( - promises.map((p = Promise.resolve()) => p.then((value) => ({ status: "fulfilled", value })).catch((reason) => ({ status: "rejected", reason })) - ) + const settled = Promise.all( + (promises || []).map((p) => Promise.resolve(p) + .then((value) => ({ status: "fulfilled", value })) + .catch((reason) => ({ status: "rejected", reason }))) ); + return applyQCompat(settled); } module.exports = allSettled; diff --git a/test/utils/utils_spec.js b/test/utils/utils_spec.js index c6108ce7..a2c0ad59 100644 --- a/test/utils/utils_spec.js +++ b/test/utils/utils_spec.js @@ -1060,7 +1060,7 @@ describe("utils", function () { }); describe("text", function () { this.timeout(TIMEOUT.MEDIUM); - var text_encoded, text_layer; + var text_encoded, text_layer, overlay_base_public_id; text_layer = "Hello World, /Nice to meet you?"; text_encoded = "Hello%20World%252C%20%252FNice%20to%20meet%20you%3F"; before(async function () { @@ -1068,9 +1068,10 @@ describe("utils", function () { // Reset, in case some other test mutated the config (which happens...) cloudinary.config(true); - // Upload sample image for overlay tests + // Upload a dedicated base image for overlay tests (do not rely on a shared "sample" resource). + overlay_base_public_id = `overlay_base_${utils.random_public_id()}`; await cloudinary.v2.uploader.upload(helper.IMAGE_FILE, { - public_id: "sample", + public_id: overlay_base_public_id, overwrite: true, tags: TEST_TAG }); @@ -1089,7 +1090,6 @@ describe("utils", function () { fs.writeFileSync(fileName, srt); - // Why do we need this? await cloudinary.v2.uploader.upload(fileName, { public_id: 'subtitles.srt', resource_type: 'raw', @@ -1189,7 +1189,10 @@ describe("utils", function () { it(`should support ${name}`, function (done) { var opt = {}; opt.overlay = options; - expect(["sample", opt]).to.produceUrl(`https://res.cloudinary.com/${cloud_name}/image/upload/l_${result}/sample`).and.emptyOptions().and.beServedByCloudinary(done); + expect([overlay_base_public_id, opt]) + .to.produceUrl(`https://res.cloudinary.com/${cloud_name}/image/upload/l_${result}/${overlay_base_public_id}`) + .and.emptyOptions() + .and.beServedByCloudinary(done); }); if (!isString(options)) { itBehavesLike("a signed url", { overlay: options }, `l_${result}`); @@ -1201,8 +1204,8 @@ describe("utils", function () { height: 100, width: 100 }; - expect(["sample", opt]) - .produceUrl(`https://res.cloudinary.com/${cloud_name}/image/upload/h_100,l_text:test_text,w_100/sample`) + expect([overlay_base_public_id, opt]) + .produceUrl(`https://res.cloudinary.com/${cloud_name}/image/upload/h_100,l_text:test_text,w_100/${overlay_base_public_id}`) .and .emptyOptions(); }); diff --git a/tools/createTestCloud.js b/tools/createTestCloud.js index 5952e01d..f70cdeaa 100644 --- a/tools/createTestCloud.js +++ b/tools/createTestCloud.js @@ -56,5 +56,6 @@ function setup() { req.end(); } - setup(); + + diff --git a/tools/scripts/test.es6.sh b/tools/scripts/test.es6.sh index 1f518a8f..1d070853 100755 --- a/tools/scripts/test.es6.sh +++ b/tools/scripts/test.es6.sh @@ -17,7 +17,7 @@ if [ "$COLLECT_COVERAGE" -eq "1" ]; then # This file should be in the config under a 'require' key # However Mocha 6 does not expose before, beforeEach after etc. at that time # When Removing support of Node 6 and 8 and using Mocha 8, we should move this to the mocharc.json file - nyc --reporter=html mocha --file "./test/setup.js" "./test/**/*spec.js" + nyc --reporter=html mocha --exit --file "./test/setup.js" "./test/**/*spec.js" exit; else echo 'Running tests on ES6 Code' @@ -26,5 +26,5 @@ else # This file should be in the config under a 'require' key # However Mocha 6 does not expose before, beforeEach after etc. at that time # When Removing support of Node 6 and 8 and using Mocha 8, we should move this to the mocharc.json file - mocha --file "./test/setup.js" "./test/**/*spec.js" + mocha --exit --file "./test/setup.js" "./test/**/*spec.js" fi diff --git a/tools/scripts/test.es6.unit.sh b/tools/scripts/test.es6.unit.sh index 5cfe0126..85e4e870 100755 --- a/tools/scripts/test.es6.unit.sh +++ b/tools/scripts/test.es6.unit.sh @@ -2,4 +2,4 @@ # This file should be in the config under a 'require' key # However Mocha 6 does not expose before, beforeEach after etc. at that time # When Removing support of Node 6 and 8 and using Mocha 8, we should move this to the mocharc.json file -mocha --file "./test/setup.js" "./test/unit/**/*spec.js" +mocha --exit --file "./test/setup.js" "./test/unit/**/*spec.js"