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

feat: encode new line characters in public-key value #623

Merged
merged 11 commits into from
Aug 24, 2022
Merged

Conversation

sitaram-kalluri
Copy link
Member

@sitaram-kalluri sitaram-kalluri commented Jul 27, 2022

- What I did

  • If the public data contains \n character, do a base64Encoding and decode when the data is fetched

- How I did it

  • In PutRequestTransformer check if the value contains \n character. If yes, encode the data using base64 and set the encoding attribute to type of encoding used.

  • In GetResponseTransformer, if the key is public or cached:public: key the data is decoded based on the metadata.encoding which represents the type of encoding used.

  • In sync_service_impl, add encoding attribute in metadata to sync the field between cloud and local secondaries and if metadata.encoding is not null, use updata:json version of regex to sync the data to cloud secondary.

  • Add unit tests to test the PutRequestTransformer and GetResponseTransformer

  • TODO: Add functional tests and e2e tests when the server changes are deployed to canary servers.

- How to verify it

  • Create a public key with new line characters and the data should be base64Encoded.

- Description for the changelog

  • Encode the new line characters in public keys

@sitaram-kalluri sitaram-kalluri changed the title feat: encode new characters in public-key value feat: encode new line characters in public-key value Jul 27, 2022
@sitaram-kalluri sitaram-kalluri requested a review from gkc July 28, 2022 14:00
Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

Tests are failing

@sitaram-kalluri
Copy link
Member Author

Tests are failing

Fixed the failing tests.

@sitaram-kalluri sitaram-kalluri requested a review from gkc July 28, 2022 17:18
if (updateVerbBuilder.value.contains('\n')) {
updateVerbBuilder.value =
AtEncoderImpl().encodeData(updateVerbBuilder.value, encodingType);
updateVerbBuilder.encoding = encodingType.toString().split('.')[1];
Copy link
Member

Choose a reason for hiding this comment

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

please check how to get name from enum in a cleaner way.
we can replace with encodingType.name using extensions
https://stackoverflow.com/questions/29567236/dart-how-to-get-the-name-of-an-enum-as-a-string

Copy link
Member Author

Choose a reason for hiding this comment

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

This change is implemented.

@murali-shris
Copy link
Member

will approve this PR once server and persistence changes are released

Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

Will approve once server changes are in prod

@sitaram-kalluri
Copy link
Member Author

Will approve once server changes are in prod

Sure Gary, Thank you.

# Conflicts:
#	at_client/pubspec.yaml
#	at_end2end_test/pubspec.yaml
#	at_functional_test/pubspec.yaml
@gkc gkc merged commit bdc34a0 into trunk Aug 24, 2022
@gkc gkc deleted the encode_public_data branch August 24, 2022 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants