fix: Update build_form function to sort signature params alphabetically#54
fix: Update build_form function to sort signature params alphabetically#54
Conversation
See: https://cloudinary.com/documentation/authentication_signatures Fixes issue of invalid signatures when using OptionalParameters that alphabetically come after timestamp
There was a problem hiding this comment.
Pull request overview
This PR fixes Cloudinary upload signature generation by ensuring all signed parameters (including timestamp) are sorted alphabetically, preventing invalid signatures when optional parameter keys come after timestamp.
Changes:
- Collects all signed
key=valueparts (excludingresource_type), addstimestamp, sorts them, and hashes the joined string plusapi_secret. - Removes incremental hashing with interleaved
&updates in favor of hashing a single canonical parameter string.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn build_form(&self, options: &BTreeSet<OptionalParameters>) -> Form { | ||
| let mut form = Form::new(); | ||
| let mut hasher = Sha1::new(); | ||
| let timestamp = Utc::now().timestamp_millis().to_string(); | ||
| let mut parts: Vec<String> = Vec::new(); | ||
|
|
||
| for option in options { | ||
| let (key, value) = option.get_pair(); | ||
| if key != "resource_type" { | ||
| hasher.update(option.to_string()); | ||
| hasher.update("&"); | ||
| parts.push(option.to_string()); | ||
| }; | ||
|
|
||
| form = form.text(key, value); | ||
| } | ||
|
|
||
| hasher.update(format!("timestamp={}{}", timestamp, self.api_secret)); | ||
| parts.push(format!("timestamp={}", timestamp)); | ||
| parts.sort(); | ||
|
|
||
| let params_string = format!("{}{}", parts.join("&"), self.api_secret); | ||
| hasher.update(¶ms_string); |
There was a problem hiding this comment.
The signature-ordering fix is critical for auth correctness, but there’s no deterministic test guarding the parameter sorting (e.g., a case where an option key alphabetically follows timestamp, like use_asset_folder_as_public_id_prefix). Consider extracting the signature param-string construction into a small helper that accepts a fixed timestamp (or injecting a clock) so you can add a unit test asserting the exact signed string / signature ordering without needing live Cloudinary credentials.
See: https://cloudinary.com/documentation/authentication_signatures
Fixes issue of invalid signatures when using OptionalParameters that alphabetically come after timestamp
(Example: Using OptionalParameters::UseAssetFolderAsPublicIdPrefix currently throws an InvalidSignature error)