Skip to content

Commit 4a805ef

Browse files
cloudinary-pkoniucloudinary-pkoniu
authored
Merge pull request #631 from cloudinary/fix-url-signing-with-encoded-chars
fix: improved calculation of the signature in url
2 parents 47e4eed + d78b737 commit 4a805ef

File tree

7 files changed

+319
-185
lines changed

7 files changed

+319
-185
lines changed

‎lib-es5/utils/index.js‎

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ var encodeDoubleArray = require('./encoding/encodeDoubleArray');
4949

5050
var config = require("../config");
5151
var generate_token = require("../auth_token");
52-
var utf8_encode = require('./utf8_encode');
5352
var crc32 = require('./crc32');
5453
var ensurePresenceOf = require('./ensurePresenceOf');
5554
var ensureOption = require('./ensureOption').defaults(config());
@@ -302,17 +301,19 @@ function process_layer(layer) {
302301
}
303302

304303
if (!isEmpty(text)) {
305-
var re = /\$\([a-zA-Z]\w*\)/g;
306-
var start = 0;
307-
var textSource = smart_escape(decodeURIComponent(text), /[,\/]/g);
308-
text = "";
309-
for (var res = re.exec(textSource); res; res = re.exec(textSource)) {
310-
text += smart_escape(textSource.slice(start, res.index));
311-
text += res[0];
312-
start = res.index + res[0].length;
313-
}
314-
text += encodeURIComponent(textSource.slice(start));
315-
components.push(text);
304+
var variablesRegex = new RegExp(/(\$\([a-zA-Z]\w+\))/g);
305+
var textDividedByVariables = text.split(variablesRegex).filter(function (x) {
306+
return x;
307+
});
308+
var encodedParts = textDividedByVariables.map(function (subText) {
309+
var matches = variablesRegex[Symbol.match](subText);
310+
var isVariable = matches ? matches.length > 0 : false;
311+
if (isVariable) {
312+
return subText;
313+
}
314+
return encodeCurlyBraces(encodeURIComponent(smart_escape(subText, new RegExp(/([,\/])/g))));
315+
});
316+
components.push(encodedParts.join(''));
316317
}
317318
} else if (type === 'fetch') {
318319
var encodedUrl = base64EncodeURL(fetchUrl);
@@ -325,6 +326,16 @@ function process_layer(layer) {
325326
return components.join(':');
326327
}
327328

329+
function replaceAllSubstrings(string, search) {
330+
var replacement = arguments.length > 2 && arguments[2] !== undefined ? arguments[2] : '';
331+
332+
return string.split(search).join(replacement);
333+
}
334+
335+
function encodeCurlyBraces(input) {
336+
return replaceAllSubstrings(replaceAllSubstrings(input, '(', '%28'), ')', '%29');
337+
}
338+
328339
/**
329340
* Parse radius options
330341
* @private
@@ -425,7 +436,8 @@ function build_upload_params(options) {
425436
quality_override: options.quality_override,
426437
accessibility_analysis: utils.as_safe_bool(options.accessibility_analysis),
427438
use_asset_folder_as_public_id_prefix: utils.as_safe_bool(options.use_asset_folder_as_public_id_prefix),
428-
visual_search: utils.as_safe_bool(options.visual_search)
439+
visual_search: utils.as_safe_bool(options.visual_search),
440+
on_success: options.on_success
429441
};
430442
return utils.updateable_resource_params(options, params);
431443
}
@@ -928,16 +940,20 @@ function url(public_id) {
928940
var to_sign = [transformation, source_to_sign].filter(function (part) {
929941
return part != null && part !== '';
930942
}).join('/');
931-
try {
932-
for (var i = 0; to_sign !== decodeURIComponent(to_sign) && i < 10; i++) {
933-
to_sign = decodeURIComponent(to_sign);
934-
}
935-
// eslint-disable-next-line no-empty
936-
} catch (error) {}
937-
var hash = compute_hash(to_sign + api_secret, signature_algorithm, 'base64');
938-
signature = hash.replace(/\//g, '_').replace(/\+/g, '-').substring(0, long_url_signature ? 32 : 8);
939-
signature = `s--${signature}--`;
943+
944+
var signatureConfig = {};
945+
if (long_url_signature) {
946+
signatureConfig.algorithm = 'sha256';
947+
signatureConfig.signatureLength = 32;
948+
} else {
949+
signatureConfig.algorithm = signature_algorithm;
950+
signatureConfig.signatureLength = 8;
951+
}
952+
953+
var truncated = compute_hash(to_sign + api_secret, signatureConfig.algorithm, 'base64').slice(0, signatureConfig.signatureLength).replace(/\//g, '_').replace(/\+/g, '-');
954+
signature = `s--${truncated}--`;
940955
}
956+
941957
var prefix = build_distribution_domain(public_id, options);
942958
var resultUrl = [prefix, resource_type, type, signature, transformation, version, public_id].filter(function (part) {
943959
return part != null && part !== '';
@@ -1156,9 +1172,8 @@ function compute_hash(input, signature_algorithm, encoding) {
11561172
if (!SUPPORTED_SIGNATURE_ALGORITHMS.includes(signature_algorithm)) {
11571173
throw new Error(`Signature algorithm ${signature_algorithm} is not supported. Supported algorithms: ${SUPPORTED_SIGNATURE_ALGORITHMS.join(', ')}`);
11581174
}
1159-
var hash = crypto.createHash(signature_algorithm);
1160-
hash.update(utf8_encode(input), 'binary');
1161-
return hash.digest(encoding);
1175+
var hash = crypto.createHash(signature_algorithm).update(input).digest();
1176+
return Buffer.from(hash).toString(encoding);
11621177
}
11631178

11641179
function clear_blank(hash) {

‎lib/utils/index.js‎

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ const encodeDoubleArray = require('./encoding/encodeDoubleArray');
3838

3939
const config = require("../config");
4040
const generate_token = require("../auth_token");
41-
const utf8_encode = require('./utf8_encode');
4241
const crc32 = require('./crc32');
4342
const ensurePresenceOf = require('./ensurePresenceOf');
4443
const ensureOption = require('./ensureOption').defaults(config());
@@ -286,17 +285,17 @@ function process_layer(layer) {
286285
}
287286

288287
if (!isEmpty(text)) {
289-
let re = /\$\([a-zA-Z]\w*\)/g;
290-
let start = 0;
291-
let textSource = smart_escape(decodeURIComponent(text), /[,\/]/g);
292-
text = "";
293-
for (let res = re.exec(textSource); res; res = re.exec(textSource)) {
294-
text += smart_escape(textSource.slice(start, res.index));
295-
text += res[0];
296-
start = res.index + res[0].length;
297-
}
298-
text += encodeURIComponent(textSource.slice(start));
299-
components.push(text);
288+
const variablesRegex = new RegExp(/(\$\([a-zA-Z]\w+\))/g);
289+
const textDividedByVariables = text.split(variablesRegex).filter(x => x);
290+
const encodedParts = textDividedByVariables.map(subText => {
291+
const matches = variablesRegex[Symbol.match](subText);
292+
const isVariable = matches ? matches.length > 0 : false;
293+
if (isVariable) {
294+
return subText;
295+
}
296+
return encodeCurlyBraces(encodeURIComponent(smart_escape(subText, new RegExp(/([,\/])/g))));
297+
});
298+
components.push(encodedParts.join(''));
300299
}
301300
} else if (type === 'fetch') {
302301
const encodedUrl = base64EncodeURL(fetchUrl);
@@ -309,6 +308,14 @@ function process_layer(layer) {
309308
return components.join(':');
310309
}
311310

311+
function replaceAllSubstrings(string, search, replacement = '') {
312+
return string.split(search).join(replacement);
313+
}
314+
315+
function encodeCurlyBraces(input) {
316+
return replaceAllSubstrings(replaceAllSubstrings(input, '(', '%28'), ')', '%29');
317+
}
318+
312319
/**
313320
* Parse radius options
314321
* @private
@@ -856,17 +863,23 @@ function url(public_id, options = {}) {
856863
let to_sign = [transformation, source_to_sign].filter(function (part) {
857864
return (part != null) && part !== '';
858865
}).join('/');
859-
try {
860-
for (let i = 0; to_sign !== decodeURIComponent(to_sign) && i < 10; i++) {
861-
to_sign = decodeURIComponent(to_sign);
862-
}
863-
// eslint-disable-next-line no-empty
864-
} catch (error) {
866+
867+
const signatureConfig = {};
868+
if (long_url_signature) {
869+
signatureConfig.algorithm = 'sha256';
870+
signatureConfig.signatureLength = 32;
871+
} else {
872+
signatureConfig.algorithm = signature_algorithm;
873+
signatureConfig.signatureLength = 8;
865874
}
866-
let hash = compute_hash(to_sign + api_secret, signature_algorithm, 'base64');
867-
signature = hash.replace(/\//g, '_').replace(/\+/g, '-').substring(0, long_url_signature ? 32 : 8);
868-
signature = `s--${signature}--`;
875+
876+
const truncated = compute_hash(to_sign + api_secret, signatureConfig.algorithm, 'base64')
877+
.slice(0, signatureConfig.signatureLength)
878+
.replace(/\//g, '_')
879+
.replace(/\+/g, '-');
880+
signature = `s--${truncated}--`;
869881
}
882+
870883
let prefix = build_distribution_domain(public_id, options);
871884
let resultUrl = [prefix, resource_type, type, signature, transformation, version, public_id].filter(function (part) {
872885
return (part != null) && part !== '';
@@ -1080,9 +1093,8 @@ function compute_hash(input, signature_algorithm, encoding) {
10801093
if (!SUPPORTED_SIGNATURE_ALGORITHMS.includes(signature_algorithm)) {
10811094
throw new Error(`Signature algorithm ${signature_algorithm} is not supported. Supported algorithms: ${SUPPORTED_SIGNATURE_ALGORITHMS.join(', ')}`);
10821095
}
1083-
let hash = crypto.createHash(signature_algorithm);
1084-
hash.update(utf8_encode(input), 'binary');
1085-
return hash.digest(encoding);
1096+
const hash = crypto.createHash(signature_algorithm).update(input).digest();
1097+
return Buffer.from(hash).toString(encoding);
10861098
}
10871099

10881100
function clear_blank(hash) {

‎test/unit/url/overlay_underlay_spec.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
const assert = require('assert');
2+
const cloudinary = require('../../../lib/cloudinary');
3+
4+
describe('URL utils with transformation parameters', () => {
5+
const CLOUD_NAME = 'test-cloud';
6+
7+
before(() => {
8+
cloudinary.config({
9+
cloud_name: CLOUD_NAME
10+
});
11+
});
12+
13+
it('should create a correct link with overlay string', () => {
14+
const url = cloudinary.utils.url('test', {
15+
overlay: {
16+
font_family: "arial",
17+
font_size: "30",
18+
text: "abc,αβγ/אבג"
19+
}
20+
});
21+
22+
assert.strictEqual(url, `http://res.cloudinary.com/${CLOUD_NAME}/image/upload/l_text:arial_30:abc%252C%CE%B1%CE%B2%CE%B3%252F%D7%90%D7%91%D7%92/test`);
23+
});
24+
25+
it('should create a correct link with underlay string', () => {
26+
const url = cloudinary.utils.url('test', {
27+
underlay: {
28+
font_family: "arial",
29+
font_size: "30",
30+
text: "abc,αβγ/אבג"
31+
}
32+
});
33+
34+
assert.strictEqual(url, `http://res.cloudinary.com/${CLOUD_NAME}/image/upload/u_text:arial_30:abc%252C%CE%B1%CE%B2%CE%B3%252F%D7%90%D7%91%D7%92/test`);
35+
});
36+
});

‎test/unit/url/sign_url_spec.js‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
const cloudinary = require('../../../lib/cloudinary');
2+
const assert = require('assert');
3+
4+
const TEST_CLOUD_NAME = 'test123';
5+
const TEST_API_KEY = '1234';
6+
const TEST_API_SECRET = 'b';
7+
8+
9+
describe("URL for authenticated asset", () => {
10+
before(function () {
11+
cloudinary.config({
12+
cloud_name: TEST_CLOUD_NAME,
13+
api_key: TEST_API_KEY,
14+
api_secret: TEST_API_SECRET
15+
});
16+
});
17+
18+
let TEST_PUBLIC_ID = 'image.jpg';
19+
it('should have signature for transformation and version', () => {
20+
const signedUrl = cloudinary.utils.url(TEST_PUBLIC_ID, {
21+
version: 1234,
22+
transformation: {
23+
crop: "crop",
24+
width: 10,
25+
height: 20
26+
},
27+
sign_url: true
28+
});
29+
30+
assert.strictEqual(signedUrl, 'http://res.cloudinary.com/test123/image/upload/s--Ai4Znfl3--/c_crop,h_20,w_10/v1234/image.jpg');
31+
});
32+
33+
it('should have signature for version', () => {
34+
const signedUrl = cloudinary.utils.url(TEST_PUBLIC_ID, {
35+
version: 1234,
36+
sign_url: true
37+
});
38+
39+
assert.strictEqual(signedUrl, 'http://res.cloudinary.com/test123/image/upload/s----SjmNDA--/v1234/image.jpg');
40+
});
41+
42+
it('should have signature for transformation', () => {
43+
const signedUrl = cloudinary.utils.url(TEST_PUBLIC_ID, {
44+
transformation: {
45+
crop: "crop",
46+
width: 10,
47+
height: 20
48+
},
49+
sign_url: true
50+
});
51+
52+
assert.strictEqual(signedUrl, 'http://res.cloudinary.com/test123/image/upload/s--Ai4Znfl3--/c_crop,h_20,w_10/image.jpg');
53+
});
54+
55+
it('should have signature for authenticated asset with transformation', () => {
56+
const signedUrl = cloudinary.utils.url(TEST_PUBLIC_ID, {
57+
type: 'authenticated',
58+
transformation: {
59+
crop: "crop",
60+
width: 10,
61+
height: 20
62+
},
63+
sign_url: true
64+
});
65+
66+
assert.strictEqual(signedUrl, 'http://res.cloudinary.com/test123/image/authenticated/s--Ai4Znfl3--/c_crop,h_20,w_10/image.jpg');
67+
});
68+
69+
it('should have signature for fetched asset', () => {
70+
const signedUrl = cloudinary.utils.url('http://google.com/path/to/image.png', {
71+
type: 'fetch',
72+
version: 1234,
73+
sign_url: true
74+
});
75+
76+
assert.strictEqual(signedUrl, 'http://res.cloudinary.com/test123/image/fetch/s--hH_YcbiS--/v1234/http://google.com/path/to/image.png');
77+
});
78+
79+
it('should have signature for authenticated asset with text overlay transformation including encoded emoji', () => {
80+
const signedUrl = cloudinary.utils.url(TEST_PUBLIC_ID, {
81+
type: 'authenticated',
82+
sign_url: true,
83+
transformation: {
84+
color: 'red',
85+
overlay: {
86+
text: 'Cool%F0%9F%98%8D',
87+
font_family: 'Times',
88+
font_size: 70,
89+
font_weight: 'bold'
90+
}
91+
}
92+
});
93+
94+
assert.strictEqual(signedUrl, 'http://res.cloudinary.com/test123/image/authenticated/s--Uqk1a-5W--/co_red,l_text:Times_70_bold:Cool%25F0%259F%2598%258D/image.jpg');
95+
});
96+
97+
it('should have signature for raw transformation string', () => {
98+
const signedUrl = cloudinary.utils.url(TEST_PUBLIC_ID, {
99+
type: 'authenticated',
100+
sign_url: true,
101+
transformation: {
102+
raw_transformation: 'co_red,l_text:Times_70_bold:Cool%25F0%259F%2598%258D'
103+
}
104+
});
105+
106+
assert.strictEqual(signedUrl, 'http://res.cloudinary.com/test123/image/authenticated/s--Uqk1a-5W--/co_red,l_text:Times_70_bold:Cool%25F0%259F%2598%258D/image.jpg');
107+
});
108+
})

0 commit comments

Comments
 (0)