Skip to content

Commit 69b0b17

Browse files
gilemosGiovanna RibeiroRubenCerna2079
authored
Fix for stored procedure empty cell bug (#2887)
## Why make this change? We have recently noticed that, if we have a column of type NVARCHAR or VARCHAR and we try to run a stored procedure that reads a row in which that column has an empty string value, we had an internal server error. This error happens when we try to run the method GetChars passing in a buffer with length 0 This PR aims to fix this problem ## What is this change? We have added a small change to the method that was throwing the exception. If we find that resultFieldSize is 0 - which means that the data in the cell we are reading has a length of 0 - we will not call the method GetChars to read the data, but assume the data is empty and return the size of the data read in bytes as 0. As you can see in the example bellow, that fixes the issue. ## How was this tested? - [x] Integration Tests - [x] Unit Tests ## Sample Request(s) We have a table with a column of type NVARCHAR called "Description". In one of the rows, Description is an empty string <img width="2258" height="494" alt="image" src="https://github.com/user-attachments/assets/857b9d93-e1e1-4c4b-b802-70693037402e" /> **Before the changes:** If we try to run a stored procedure that reads that empty cell, we get an error <img width="2783" height="1245" alt="image" src="https://github.com/user-attachments/assets/adc80578-2532-4f71-b781-f4bee8798334" /> **After changes** Stored procedure runs as expected <img width="2776" height="1214" alt="image" src="https://github.com/user-attachments/assets/bd4e0e2d-de34-4a20-8805-7a676f40de15" /> --------- Co-authored-by: Giovanna Ribeiro <[email protected]> Co-authored-by: RubenCerna2079 <[email protected]>
1 parent d6d0f83 commit 69b0b17

11 files changed

Lines changed: 187 additions & 98 deletions

File tree

‎src/Core/Resolvers/QueryExecutor.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,12 @@ internal int StreamCharData(DbDataReader dbDataReader, long availableSize, Strin
740740
// else we throw exception.
741741
ValidateSize(availableSize, resultFieldSize);
742742

743+
// If the cell is empty, don't append anything to the resultJsonString and return 0.
744+
if (resultFieldSize == 0)
745+
{
746+
return 0;
747+
}
748+
743749
char[] buffer = new char[resultFieldSize];
744750

745751
// read entire field into buffer and reduce available size.
@@ -766,6 +772,13 @@ internal int StreamByteData(DbDataReader dbDataReader, long availableSize, int o
766772
// else we throw exception.
767773
ValidateSize(availableSize, resultFieldSize);
768774

775+
// If the cell is empty, set resultBytes to an empty array and return 0.
776+
if (resultFieldSize == 0)
777+
{
778+
resultBytes = Array.Empty<byte>();
779+
return 0;
780+
}
781+
769782
resultBytes = new byte[resultFieldSize];
770783

771784
dbDataReader.GetBytes(ordinal: ordinal, dataOffset: 0, buffer: resultBytes, bufferOffset: 0, length: resultBytes.Length);

‎src/Service.Tests/DatabaseSchema-DwSql.sql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,8 @@ VALUES (1, 'Awesome book', 1234),
336336
(17, 'CONN%_CONN', 1234),
337337
(18, '[Special Book]', 1234),
338338
(19, 'ME\YOU', 1234),
339-
(20, 'C:\\LIFE', 1234);
339+
(20, 'C:\\LIFE', 1234),
340+
(21, '', 1234);
340341

341342
INSERT INTO book_website_placements(id, book_id, price) VALUES (1, 1, 100), (2, 2, 50), (3, 3, 23), (4, 5, 33);
342343

‎src/Service.Tests/DatabaseSchema-MsSql.sql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,8 @@ VALUES (1, 'Awesome book', 1234),
531531
(17, 'CONN%_CONN', 1234),
532532
(18, '[Special Book]', 1234),
533533
(19, 'ME\YOU', 1234),
534-
(20, 'C:\\LIFE', 1234);
534+
(20, 'C:\\LIFE', 1234),
535+
(21, '', 1234);
535536
SET IDENTITY_INSERT books OFF
536537

537538
SET IDENTITY_INSERT books_mm ON

‎src/Service.Tests/DatabaseSchema-MySql.sql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,8 @@ INSERT INTO books(id, title, publisher_id)
388388
(17, 'CONN%_CONN', 1234),
389389
(18, '[Special Book]', 1234),
390390
(19, 'ME\\YOU', 1234),
391-
(20, 'C:\\\\LIFE', 1234);
391+
(20, 'C:\\\\LIFE', 1234),
392+
(21, '', 1234);
392393
INSERT INTO book_website_placements(book_id, price) VALUES (1, 100), (2, 50), (3, 23), (5, 33);
393394
INSERT INTO website_users(id, username) VALUES (1, 'George'), (2, NULL), (3, ''), (4, 'book_lover_95'), (5, 'null');
394395
INSERT INTO book_author_link(book_id, author_id) VALUES (1, 123), (2, 124), (3, 123), (3, 124), (4, 123), (4, 124), (5, 126);

‎src/Service.Tests/DatabaseSchema-PostgreSql.sql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,8 @@ INSERT INTO books(id, title, publisher_id)
391391
(17, 'CONN%_CONN', 1234),
392392
(18, '[Special Book]', 1234),
393393
(19, 'ME\YOU', 1234),
394-
(20, 'C:\\LIFE', 1234);
394+
(20, 'C:\\LIFE', 1234),
395+
(21, '', 1234);
395396
INSERT INTO book_website_placements(book_id, price) VALUES (1, 100), (2, 50), (3, 23), (5, 33);
396397
INSERT INTO website_users(id, username) VALUES (1, 'George'), (2, NULL), (3, ''), (4, 'book_lover_95'), (5, 'null');
397398
INSERT INTO book_author_link(book_id, author_id) VALUES (1, 123), (2, 124), (3, 123), (3, 124), (4, 123), (4, 124), (5, 126);;

‎src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ public async Task TestStoredProcedureMutationForDeletion(string dbQueryToVerifyD
257257

258258
string currentDbResponse = await GetDatabaseResultAsync(dbQueryToVerifyDeletion);
259259
JsonDocument currentResult = JsonDocument.Parse(currentDbResponse);
260-
Assert.AreEqual(currentResult.RootElement.GetProperty("maxId").GetInt64(), 20);
260+
Assert.AreEqual(currentResult.RootElement.GetProperty("maxId").GetInt64(), 21);
261261
JsonElement graphQLResponse = await ExecuteGraphQLRequestAsync(graphQLMutation, graphQLMutationName, isAuthenticated: true);
262262

263263
// Stored Procedure didn't return anything
@@ -266,7 +266,7 @@ public async Task TestStoredProcedureMutationForDeletion(string dbQueryToVerifyD
266266
// check to verify new element is inserted
267267
string updatedDbResponse = await GetDatabaseResultAsync(dbQueryToVerifyDeletion);
268268
JsonDocument updatedResult = JsonDocument.Parse(updatedDbResponse);
269-
Assert.AreEqual(updatedResult.RootElement.GetProperty("maxId").GetInt64(), 19);
269+
Assert.AreEqual(updatedResult.RootElement.GetProperty("maxId").GetInt64(), 20);
270270
}
271271

272272
public async Task InsertMutationOnTableWithTriggerWithNonAutoGenPK(string dbQuery)

‎src/Service.Tests/SqlTests/GraphQLPaginationTests/GraphQLPaginationTestBase.cs‎

Lines changed: 101 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -84,95 +84,101 @@ public async Task RequestMaxUsingNegativeOne()
8484
}
8585
}";
8686

87-
// this resultset represents all books in the db.
88-
JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false);
8987
string expected = @"{
90-
""items"": [
88+
""items"": [
9189
{
92-
""id"": 1,
93-
""title"": ""Awesome book""
90+
""id"": 1,
91+
""title"": ""Awesome book""
9492
},
9593
{
96-
""id"": 2,
97-
""title"": ""Also Awesome book""
94+
""id"": 2,
95+
""title"": ""Also Awesome book""
9896
},
9997
{
100-
""id"": 3,
101-
""title"": ""Great wall of china explained""
98+
""id"": 3,
99+
""title"": ""Great wall of china explained""
102100
},
103101
{
104-
""id"": 4,
105-
""title"": ""US history in a nutshell""
102+
""id"": 4,
103+
""title"": ""US history in a nutshell""
106104
},
107105
{
108-
""id"": 5,
109-
""title"": ""Chernobyl Diaries""
106+
""id"": 5,
107+
""title"": ""Chernobyl Diaries""
110108
},
111109
{
112-
""id"": 6,
113-
""title"": ""The Palace Door""
110+
""id"": 6,
111+
""title"": ""The Palace Door""
114112
},
115113
{
116-
""id"": 7,
117-
""title"": ""The Groovy Bar""
114+
""id"": 7,
115+
""title"": ""The Groovy Bar""
118116
},
119117
{
120-
""id"": 8,
121-
""title"": ""Time to Eat""
118+
""id"": 8,
119+
""title"": ""Time to Eat""
122120
},
123121
{
124-
""id"": 9,
125-
""title"": ""Policy-Test-01""
122+
""id"": 9,
123+
""title"": ""Policy-Test-01""
126124
},
127125
{
128-
""id"": 10,
129-
""title"": ""Policy-Test-02""
126+
""id"": 10,
127+
""title"": ""Policy-Test-02""
130128
},
131129
{
132-
""id"": 11,
133-
""title"": ""Policy-Test-04""
130+
""id"": 11,
131+
""title"": ""Policy-Test-04""
134132
},
135133
{
136-
""id"": 12,
137-
""title"": ""Time to Eat 2""
134+
""id"": 12,
135+
""title"": ""Time to Eat 2""
136+
},
137+
{
138+
""id"": 13,
139+
""title"": ""Before Sunrise""
138140
},
139141
{
140-
""id"": 13,
141-
""title"": ""Before Sunrise""
142+
""id"": 14,
143+
""title"": ""Before Sunset""
142144
},
143145
{
144-
""id"": 14,
145-
""title"": ""Before Sunset""
146+
""id"": 15,
147+
""title"": ""SQL_CONN""
146148
},
147149
{
148-
""id"": 15,
149-
""title"": ""SQL_CONN""
150+
""id"": 16,
151+
""title"": ""SOME%CONN""
150152
},
151153
{
152-
""id"": 16,
153-
""title"": ""SOME%CONN""
154+
""id"": 17,
155+
""title"": ""CONN%_CONN""
154156
},
155157
{
156-
""id"": 17,
157-
""title"": ""CONN%_CONN""
158+
""id"": 18,
159+
""title"": ""[Special Book]""
158160
},
159161
{
160-
""id"": 18,
161-
""title"": ""[Special Book]""
162+
""id"": 19,
163+
""title"": ""ME\\YOU""
162164
},
163165
{
164-
""id"": 19,
165-
""title"": ""ME\\YOU""
166+
""id"": 20,
167+
""title"": ""C:\\\\LIFE""
166168
},
167169
{
168-
""id"": 20,
169-
""title"": ""C:\\\\LIFE""
170+
""id"": 21,
171+
""title"": """"
170172
}
171-
],
172-
""endCursor"": null,
173-
""hasNextPage"": false
173+
],
174+
""endCursor"": null,
175+
""hasNextPage"": false
174176
}";
175177

178+
// Note: The max page size is 21 for MsSql and 20 for all other data sources, so when using -1
179+
// this resultset represents all books in the db.
180+
JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false);
181+
176182
SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString());
177183
}
178184

@@ -196,91 +202,96 @@ public async Task RequestNoParamFullConnection()
196202
}";
197203

198204
JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false);
205+
199206
string expected = @"{
200-
""items"": [
207+
""items"": [
201208
{
202-
""id"": 1,
203-
""title"": ""Awesome book""
209+
""id"": 1,
210+
""title"": ""Awesome book""
204211
},
205212
{
206-
""id"": 2,
207-
""title"": ""Also Awesome book""
213+
""id"": 2,
214+
""title"": ""Also Awesome book""
208215
},
209216
{
210-
""id"": 3,
211-
""title"": ""Great wall of china explained""
217+
""id"": 3,
218+
""title"": ""Great wall of china explained""
212219
},
213220
{
214-
""id"": 4,
215-
""title"": ""US history in a nutshell""
221+
""id"": 4,
222+
""title"": ""US history in a nutshell""
216223
},
217224
{
218-
""id"": 5,
219-
""title"": ""Chernobyl Diaries""
225+
""id"": 5,
226+
""title"": ""Chernobyl Diaries""
220227
},
221228
{
222-
""id"": 6,
223-
""title"": ""The Palace Door""
229+
""id"": 6,
230+
""title"": ""The Palace Door""
224231
},
225232
{
226-
""id"": 7,
227-
""title"": ""The Groovy Bar""
233+
""id"": 7,
234+
""title"": ""The Groovy Bar""
228235
},
229236
{
230-
""id"": 8,
231-
""title"": ""Time to Eat""
237+
""id"": 8,
238+
""title"": ""Time to Eat""
232239
},
233240
{
234-
""id"": 9,
235-
""title"": ""Policy-Test-01""
241+
""id"": 9,
242+
""title"": ""Policy-Test-01""
236243
},
237244
{
238-
""id"": 10,
239-
""title"": ""Policy-Test-02""
245+
""id"": 10,
246+
""title"": ""Policy-Test-02""
240247
},
241248
{
242-
""id"": 11,
243-
""title"": ""Policy-Test-04""
249+
""id"": 11,
250+
""title"": ""Policy-Test-04""
244251
},
245252
{
246-
""id"": 12,
247-
""title"": ""Time to Eat 2""
253+
""id"": 12,
254+
""title"": ""Time to Eat 2""
248255
},
249256
{
250-
""id"": 13,
251-
""title"": ""Before Sunrise""
257+
""id"": 13,
258+
""title"": ""Before Sunrise""
252259
},
253260
{
254-
""id"": 14,
255-
""title"": ""Before Sunset""
261+
""id"": 14,
262+
""title"": ""Before Sunset""
256263
},
257264
{
258-
""id"": 15,
259-
""title"": ""SQL_CONN""
265+
""id"": 15,
266+
""title"": ""SQL_CONN""
260267
},
261268
{
262-
""id"": 16,
263-
""title"": ""SOME%CONN""
269+
""id"": 16,
270+
""title"": ""SOME%CONN""
264271
},
265272
{
266-
""id"": 17,
267-
""title"": ""CONN%_CONN""
273+
""id"": 17,
274+
""title"": ""CONN%_CONN""
268275
},
269276
{
270-
""id"": 18,
271-
""title"": ""[Special Book]""
277+
""id"": 18,
278+
""title"": ""[Special Book]""
272279
},
273280
{
274-
""id"": 19,
275-
""title"": ""ME\\YOU""
281+
""id"": 19,
282+
""title"": ""ME\\YOU""
276283
},
277284
{
278-
""id"": 20,
279-
""title"": ""C:\\\\LIFE""
285+
""id"": 20,
286+
""title"": ""C:\\\\LIFE""
287+
},
288+
{
289+
""id"": 21,
290+
""title"": """"
280291
}
281-
],
282-
""endCursor"": null,
283-
""hasNextPage"": false
292+
],
293+
""endCursor"": null,
294+
""hasNextPage"": false
284295
}";
285296

286297
SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString());

‎src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,22 @@ SELECT title FROM books
268268
await QueryWithSingleColumnPrimaryKey(msSqlQuery);
269269
}
270270

271+
[TestMethod]
272+
public virtual async Task QueryWithEmptyStringResult()
273+
{
274+
string graphQLQueryName = "book_by_pk";
275+
string graphQLQuery = @"{
276+
book_by_pk(id: 21) {
277+
title
278+
}
279+
}";
280+
281+
JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false);
282+
283+
string title = actual.GetProperty("title").GetString();
284+
Assert.AreEqual("", title);
285+
}
286+
271287
[TestMethod]
272288
public async Task QueryWithSingleColumnPrimaryKeyAndMappings()
273289
{

0 commit comments

Comments
 (0)