-
Notifications
You must be signed in to change notification settings - Fork 412
fix(data-connect): correctly serialize strings with special characters #3056
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -928,4 +928,70 @@ describe('DataConnectApiClient CRUD helpers', () => { | |
| .to.be.rejectedWith(FirebaseDataConnectError, `${serverErrorString}. ${additionalErrorMessageForBulkImport}`); | ||
| }); | ||
| }); | ||
|
|
||
| describe('Issue #3043: String serialization', () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another nit: the comments above check expects are mostly unnecessary. the code itself describes what the comments say in plain language |
||
| it('should correctly escape special characters in strings during insert', async () => { | ||
| const data = { | ||
| content: 'Line 1\nLine 2', | ||
| }; | ||
|
|
||
| await apiClient.insert(tableName, data); | ||
| const callArgs = executeGraphqlStub.firstCall.args[0]; | ||
|
|
||
| // Expected part of the query: content: "Line 1\nLine 2" | ||
| // which means the string "Line 1\\nLine 2" should be present in the call args. | ||
| expect(callArgs).to.include('content: "Line 1\\nLine 2"'); | ||
| }); | ||
|
|
||
| it('should correctly escape backslash', async () => { | ||
| const data = { | ||
| content: 'Backslash \\', | ||
| }; | ||
|
|
||
| await apiClient.insert(tableName, data); | ||
| const callArgs = executeGraphqlStub.firstCall.args[0]; | ||
|
|
||
| // "Backslash \\" | ||
| // Escaped for GraphQL: "Backslash \\\\" | ||
| expect(callArgs).to.include('content: "Backslash \\\\"'); | ||
| }); | ||
|
|
||
| it('should correctly escape double quotes', async () => { | ||
| const data = { | ||
| content: 'Quote "test"', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| }; | ||
|
|
||
| await apiClient.insert(tableName, data); | ||
| const callArgs = executeGraphqlStub.firstCall.args[0]; | ||
|
|
||
| // "Quote \"test\"" | ||
| // Escaped for GraphQL: "Quote \\"test\\"" | ||
| expect(callArgs).to.include('content: "Quote \\"test\\""'); | ||
| }); | ||
|
|
||
| it('should correctly escape tab character', async () => { | ||
| const data = { | ||
| content: 'Tab\tCharacter', | ||
| }; | ||
|
|
||
| await apiClient.insert(tableName, data); | ||
| const callArgs = executeGraphqlStub.firstCall.args[0]; | ||
|
|
||
| // "Tab\tCharacter" | ||
| // Escaped for GraphQL: "Tab\\tCharacter" | ||
| expect(callArgs).to.include('content: "Tab\\tCharacter"'); | ||
| }); | ||
|
|
||
| it('should correctly handle emojis', async () => { | ||
| const data = { | ||
| content: 'Emoji 😊', | ||
| }; | ||
|
|
||
| await apiClient.insert(tableName, data); | ||
| const callArgs = executeGraphqlStub.firstCall.args[0]; | ||
|
|
||
| // "Emoji 😊" | ||
| expect(callArgs).to.include('content: "Emoji 😊"'); | ||
| }); | ||
| }); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add more tests, one featuring backslash, one with double quotes, one with the tab charater, and one with an emoji
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added tests for backslash, double quotes, tab character, and emoji. |
||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: probably remove the mention of issue #3043 in the describe string