Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Handle unencrypted message while getting messages with crypto #120

Merged
merged 8 commits into from
Nov 27, 2023

Conversation

mohitpubnub
Copy link
Contributor

fix: Handle unencrypted message while getting messages with cryptoModule configured.

Handle unencrypted message while getting messages with cryptoModule configured.

@mohitpubnub mohitpubnub self-assigned this Nov 24, 2023
@mohitpubnub mohitpubnub added priority: medium This PR should be reviewed after all high priority PRs. type: fix This PR contains fixes to existing features. type: test This PR contains new tests for existing functionality or fixes to existing tests. type: refactor This PR contains refactored existing features. labels Nov 24, 2023
@@ -154,25 +154,46 @@ class BatchHistoryResultEntry {
/// Otherwise, it will be `null`.
Map<String, dynamic>? meta;

/// Error message. this will contain information if message decryption is failed
/// for given `message`.
String? error;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you have custom class/type that describes an error? If there's no such a class, let's leave it as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

message = object['message'];
} else {
try {
message = decryptFunction is decryptWithKey

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, what's the logic behind it?

Copy link

@jguz-pubnub jguz-pubnub Nov 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, now I see that you could pass decrypt(...) or decryptWithKey(...) function from ICryptoModule. Perhaps there's no way to split deserialization and decryption into two separate concerns, this would require significant changes. What I mean by this is parsing the objects as they came without introducing decryption there. This could be a separate layer that takes BatchHistoryResultEntry or whatever lightweight DAO object created in the previous parsing step, decrypts its payload, and returns the decrypted result back to the caller. It's not possible to do it right now so let's leave it as it is, but anyway I wanted to mention about other possibilities :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed!
Need to deal with this till the time we completely stop supporting explicit cipherKey passing mechanism.

@mohitpubnub
Copy link
Contributor Author

@pubnub-release-bot release dart as v4.3.1

@mohitpubnub mohitpubnub merged commit 4287eaf into master Nov 27, 2023
5 checks passed
@mohitpubnub mohitpubnub deleted the CLEN-1719 branch November 27, 2023 15:38
@pubnub-release-bot
Copy link
Contributor

🚀 Release successfully completed 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: medium This PR should be reviewed after all high priority PRs. type: fix This PR contains fixes to existing features. type: refactor This PR contains refactored existing features. type: test This PR contains new tests for existing functionality or fixes to existing tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants