Skip to content

Commit 0120875

Browse files
authored
Fix encoding of publicId
* Fix incorrect encoding on toURL
1 parent 506e830 commit 0120875

File tree

2 files changed

+28
-14
lines changed

2 files changed

+28
-14
lines changed

__TESTS__/unit/url/encoding.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,13 @@ describe('Tests for Encoding the URL', () => {
1919
it('Should encode cloudinary characters (",") in a publicID', () => {
2020
const url = createNewImage('sam,ple')
2121
.toURL();
22-
expect(url).toBe('https://res.cloudinary.com/demo/image/upload/sam%252Cple');
22+
expect(url).toBe('https://res.cloudinary.com/demo/image/upload/sam%2Cple');
23+
});
24+
25+
it('Should encode cloudinary characters ("☺") in a publicID', () => {
26+
const url = createNewImage('sample☺')
27+
.toURL();
28+
expect(url).toBe('https://res.cloudinary.com/demo/image/upload/sample%E2%98%BA');
2329
});
2430

2531
it('Does not mutate valid / in publicID', () => {

src/assets/CloudinaryFile.ts

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -264,24 +264,32 @@ class CloudinaryFile {
264264
const prefix = getUrlPrefix(this.cloudName, this.urlConfig);
265265
const transformationString = transformation ? transformation.toString() : '';
266266
const version = getUrlVersion(this.publicID, this.version, this.urlConfig.forceVersion);
267+
const publicID = this.publicID;
267268

268-
const publicID = this.publicID
269-
// Serialize the publicID, but leave slashes alone.
270-
// we can't use serializeCloudinaryCharacters because that does both things (, and /)
271-
.replace(/,/g, '%2C');
269+
if (typeof transformation === 'string') {
272270

273-
// Resource type is a mixture of assetType, deliveryType and various URL Configurations
274-
// Note how `suffix` changes both image/upload (resourceType) and also is appended at the end
275-
const url = [prefix, this.getResourceType(), this.getSignature(), transformationString, version, publicID, this.suffix]
276-
.filter((a) => a)
277-
.join('/');
271+
const url = [prefix, this.getResourceType(), this.getSignature(), transformationString, version, publicID.replace(/,/g, '%2C'), this.suffix]
272+
.filter((a) => a)
273+
.join('/');
278274

279-
if (typeof transformation === 'string') {
280275
return url;
281276
} else {
282-
const safeURL = encodeURI(url)
283-
.replace(/\?/g, '%3F')
284-
.replace(/=/g, '%3D');
277+
// Avoid applying encodeURI on entire string in case where we have transformations with comma (i.e. f_auto,q_auto)
278+
// Since encodeURI does not encode commas we replace commas *only* on the publicID
279+
const safeURL =
280+
[
281+
encodeURI(prefix),
282+
this.getResourceType(),
283+
this.getSignature(),
284+
encodeURI(transformationString),
285+
version,
286+
encodeURI(publicID).replace(/,/g, '%2C'),
287+
this.suffix && encodeURI(this.suffix)
288+
]
289+
.filter((a) => a)
290+
.join('/')
291+
.replace(/\?/g, '%3F')
292+
.replace(/=/g, '%3D');
285293

286294
const queryParams = new URLSearchParams(this.urlConfig.queryParams as Record<string, string>);
287295

0 commit comments

Comments
 (0)