chore: url parse replaced by native url + multiple fixes and rewrites

Author: cloudinary-pkoniu

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,19 @@ 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) : ''
+  };
+
+  // Only add port if not default (to match url.parse() behavior)
+  if (parsedUrl.port) {
+    request_options.port = parsedUrl.port;
+  }
 
   request_options = extend(request_options, {
     method: method,
@@ -65,10 +77,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; }
     }

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,31 +48,39 @@ 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])
     });
   }
 
   return conf;
 }
 
 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 uri = new URL(ENV_URL);
+  if (uri.search) {
+    const query = {};
+    uri.searchParams.forEach((value, key) => {
+      query[key] = value;
+    });
+    entries(query).forEach(([key, value]) => putNestedValue(confToExtend, key, value));
   }
 }
 

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,21 @@ 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);
+  // Use WHATWG URL API instead of deprecated url.parse()
+  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) : ''
+  };
+
+  // Only add port if not default (to match url.parse() behavior)
+  if (parsedUrl.port) {
+    post_options.port = parsedUrl.port;
+  }
+
   let headers = {
     'Content-Type': `multipart/form-data; boundary=${boundary}`,
     'User-Agent': utils.getUserAgent()

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,10 @@ 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;
+    // Use WHATWG URL API instead of deprecated url.parse()
+    // 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 +1664,8 @@ function deferredPromise() {
     reject = _reject;
   });
   applyQCompat(promise);
+  promise.catch(() => {
+  });
   return {
     resolve,
     reject,

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,19 +32,19 @@ 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",
       oauth_token: undefined
     });
     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,19 +91,20 @@ 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",
       oauth_token: undefined
     });
     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,15 +115,16 @@ 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,
       oauth_token: undefined
     });
     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');
     });
   });
 });