C# Web API - how to handle connection during continues stream flushing

Question

I have this follow class that responsible to stream a huge list continuously (1000 records per flush)

What happened if the user close the browser while the connection still open and the loop not ended yet? Will the connection stay open? And why I get "This async method lack await operator"?

public async Task<HttpResponseMessage> SendList(int id)
    {
        var response = Request.CreateResponse();

        response.Content = new PushStreamContent(async (stream, Content, context) =>
        {
            try
            {
                using (SqlConnection myConnection = new SqlConnection())
                using (SqlCommand sqlCmd = new SqlCommand())
                {
                    string conn = ConfigurationManager.ConnectionStrings[SRVTarget].ConnectionString;
                    myConnection.ConnectionString = conn;
                    sqlCmd.CommandType = System.Data.CommandType.Text;
                    sqlCmd.CommandText = string.Concat("SELECT * FROM  [dbo].[fGetList](",id,")");
                    sqlCmd.Connection = myConnection;
                    myConnection.Open();

                    SqlDataReader reader = null;
                    reader = await sqlCmd.ExecuteReaderAsync();
                    string empl;
                    int j = 1;
                    string emp = string.Empty;
                    while (await reader.ReadAsync())
                    {
                        empl = string.Empty;
                        for (int i = 0; i < reader.FieldCount; i++)
                        {
                            empl = String.Concat(empl, reader.GetValue(i).ToString());
                            empl = String.Concat(empl, ',');
                        }
                        emp = String.Concat(emp, empl.Substring(0, empl.Length - 1), Environment.NewLine);
                        if (j % 1000 == 0 | reader.Read() == false)
                        {
                            using (var writer = new StreamWriter(stream, System.Text.Encoding.UTF8))
                            {
                                await writer.WriteAsync(emp);
                                await writer.FlushAsync();
                            }
                            emp = string.Empty;
                        };
                        j = j + 1;
                    }
                    myConnection.Close();
                }
            }
            finally
            {
                stream.Close();

            }

        });

        return response;
    }

NEVER EVER EVER build up a string like that and execute it. You need to use parameterized queries before sql injection wreaks havoc. You also really should avoid using select *, only select the columns you need. To read more about sql injection check this. http://bobby-tables.com/ — Sean Lange, May 22 '18 at 14:12
Relax, this is just an example (I don't actually use *) and this is a call to function, not table. — Yaniv Ben Yohana, May 22 '18 at 15:40
Doesn't matter if it is a function or a query against a table, the pattern here is vulnerable to sql injection. Sure in this case it is an int but what if somebody else uses copy/paste of your code as a working example? — Sean Lange, May 22 '18 at 15:49
User-defined Function get receive only defined parameters. id in this example is int. again, this is just example, it don't contain the real name of the sql function, not the connection string, not the authentication method. And please answer my question. If you can't than please stop suggest irrelevant comments. — Yaniv Ben Yohana, May 22 '18 at 16:36
Best of luck mate. Sql injection is real and the pattern you have is vulnerable. It has nothing to do with you using a function. Concatenating strings is horrible practice and at some point you will most likely suffer from sql injection. It is so simple to fix but you don't want to accept that. — Sean Lange, May 22 '18 at 17:41
For the questions at hand, since the web is a connectionless environment what do you think will happen if the user closes their browser? Remember that the browser is waiting for a response from the server, the server doesn't know the user is there. The second question, I don't understand the issue. Is that a compiler error or a run time error? — Sean Lange, May 22 '18 at 17:44
Maybe try looking here. https://stackoverflow.com/questions/29923215/should-i-worry-about-this-async-method-lacks-await-operators-and-will-run-syn or even here. https://www.google.com/search?q=This+async+method+lack+await+operator&ie=utf-8&oe=utf-8&client=firefox-b-1 — Sean Lange, May 22 '18 at 17:44
SQL injection can't append here. [fGetList] is the only object that is permitted in select (Update/delete/insert are not permitted). More over, the code inside the function is "select * from someTable where id=@id" About the await - thanks, your comment give me what I was looking for. About the connection - As you can see I'm streaming the data until I finish reading it from the SQL Server. How can I know if the client is still listening (receiving the stream)? — Yaniv Ben Yohana, May 22 '18 at 19:18
Ugh....when you build up a string and execute it the code is vulnerable plain and simple. And the pattern is just sloppy and lazy. But if you don't want to use parameters don't be shocked when it happens. — Sean Lange, May 22 '18 at 19:20
You can't know if the client is still listening as far as I know. Web traffic is stateless so it would be impossible to know for sure. — Sean Lange, May 22 '18 at 19:21
Enlighten me how this function can be SQL injected if it the only object permitted in select read via the connection string: ALTER FUNCTION [dbo].[fGetList] ( @ID AS INT, ) RETURNS TABLE AS RETURN ( SELECT Todo.username, Todo.Whattodo,Todo.subID From sometable Todo Where Todo.ID=@ID ) — Yaniv Ben Yohana, May 23 '18 at 06:22
You don't seem to want put any effort into understanding sql injection. The function isn't the issue. The problem is building up a string and sending that to your sql server. That is the very definition of how sql injection works. Again, it isn't the function, it is the code you are using when calling the function. Your coding style of not using parameters is going to bite you one day. It is not a matter of if, it is a matter of when. — Sean Lange, May 23 '18 at 13:23
I do understand that if no other members of the SQL database are permitted (no view or table, no update, insert or delete) that no matter what the user will try to do, he can't do nothing except calling fGetList Function. Method that I'm using is to call only functions and store-procedures with very defined permissions inside the SQL — Yaniv Ben Yohana, May 23 '18 at 16:50
That is great for this exact piece of code. But the pattern you are using is the problem. What happens when somebody copies your code and now they are totally exposed. It is SO simple to use parameters that not doing it borders on negligence. I will post an answer to show you how incredibly simple it is. Sure this off topic of your original post but preventing sql injection is paramount to a good application. The fact that it still happens these days is shocking. — Sean Lange, May 23 '18 at 18:53

score 0 · Accepted Answer · answered May 23 '18 at 18:54

0

Modifying your code to use parameters is super simple. Continuing to not use parameters is just a terrible habit. Here is the same code but it is 100% safe from sql injection no matter how you slice it or what permissions the user has.

sqlCmd.CommandText = string.Concat("SELECT * FROM  [dbo].[fGetList](@id)");
sqlCmd.Parameters.Add("@id", SqlDbType.Int).Value = ID;

answered May 23 '18 at 18:54

Sean Lange

33,028
3
25
40

How can one sql injection my code? The controller is only allowed one parameter (id) trough Task SendList(int id) – Yaniv Ben Yohana May 27 '18 at 07:37

C# Web API - how to handle connection during continues stream flushing

1 Answers1