Avoid String Concatenation to Create Queries

Avoid string concatenation to create queries

While there might be usecases where you build a prepared statement by string-concatenation before compiling it, it is always bad practice to insert query-parameters using string-concatenation for two reasons:

1. Performance: When using a prepared statement the query-syntax has to be parsed only once and the access-path has to be calculated only once for each distinct query-type. When building statements by string-concatenation parsing and optimizing has to be done for each execution of the query.

2. Security: Using string-concatenation with data provided by the user is always prone to SQL-injection-attacks. Suppose you got a statement:

   query = "select secret_data from users where userid = '" + userid_param + "'";  
   

And imagine someone sends a userid_param containing "' OR 1=1;"...

This way the only way to defend is doing 100% correct input-sanitation which might be quite hard to get right depending on the language used. When using prepared statements with a properly implemented driver the driver will isolate the statement form the query-parameters so nothing will be mixed up.

Why is concatenating SQL strings a bad idea?

Short answer: building queries by concatenating strings usually allows SQL injection.

Imagine that someone tries to create a user with the name "Bob, Joe, 12345); DROP TABLE workers; --". You end up building the query "Insert INTO workers Values(Bob, Joe, 12345); DROP TABLE workers; --name, 34345236);" Bye-bye database. SQL injection can also lead to queries returning data that they shouldn't, such as password tables. Any time that you have SQL injection, just assume that you're allowing arbitrary third parties to issue arbitrary commands to your database.

The AddWithValue() approach is called "parametrized queries". It results in very similar commands being generated, but AddWithValue() takes care of any weird stuff like spaces and quotes in the parameter values that could cause your command to mean something other than what you want it to mean. Sure, you could do that escaping manually, but it can be tricky to get correct. It's much easier and safer to let the library handle it for you.

Obligatory XKCD

Note that I don't mean that the library is actually escaping the strings that you give it when you create a parameterized query. It could, but I'm not aware of any SQL libraries that take that approach -- usually, the SQL engine has special support for parameterized queries. This allows parameterized queries to be more efficient than ad-hoc queries: the SQL engine can pre-compile the SQL statement leaving only field values to be filled in at run-time.

Is there a better way than string concatenation when implementing dynamic variables into an SQL statement

A good alternative is using prepared statements :
Example

sql= "INSERT INTO imt_database.Comment(error_id,user,content) VALUES (?,?,?);";
        try{
            Class.forName("com.mysql.jdbc.Driver");
            conn = DriverManager.getConnection(URL,"root","toor");
            PreparedStatement ps = conn.prepareStatement(sql);
            ps.setString(1, Error_id);
            ps.setString(2, User);
            ps.setString(3, Content);
            ps.executeUpdate();
        }catch(Exception e)

Does String concatenation in database query cause performance issues?

We ended up going for a very different solution, running one Query per row to be updated.
Now I feel stupid and the guy at DB who did the QA of my Query who asked the question should be ashamed.

The column names FIRSTNAME LASTNAME etc. are of course all implicitly a part of the one and same TABLE ROW. So they can be written as THESAMEROW.FIRSTNAME, THESAMEROW.LASTNAME etc. Looking at it this way, only one string will of course be constructed per row in the database.

My worry that N^4 strings would be created is therefore completely wrong, and my Query would scale perfectly linear on a larger database.

SQL Injection prevention for concatenated string inside IN clause

As a quick and partial (we assume TABLEID field be of type NUMBER) solution you can verify that each item in sa is a valid integer:

private string concatenateStrings(string[] sa) {
   return string.Join(", ", sa
     .Where(item => Regex.IsMatch(item, @"^\-?[0-9]+$"))); 
} 

public void UpdateClaimSts(string[] ids) {
  string query = string.Format(
    @"UPDATE MYTABLE
         SET STATUS = 'X'
       WHERE TABLEID IN ({0})", concatenateStrings(ids));
      ...

In general case, you can try using bind variables (please, notice plural: we have to create many of them):

public void UpdateClaimSts(string[] ids) {  
  // :id_0, :id_1, ..., :id_N   
  string bindVariables = string.Join(", ", ids
    .Select((id, index) => ":id_" + index.ToString()));

  string query = string.Format(
    @"UPDATE MYTABLE
         SET STATUS = 'X'
       WHERE TABLEID IN ({0})", bindVariables);

  // Do not forget to wrap IDisposable into "using"
  using (OracleCommand dbCommand = ...) {
    ...
    // Each item of the ids should be assigned to its bind variable
    for (int i = 0; i < ids.Length; ++i)
      dbCommand.Parameters.Add(":id_" + i.ToString(), OracleType.VarChar).Value = ids[i];

   ...

Swift prevent Optional( ) or nil in string concatenate result | Create query string from object

The best practice is to use the URLComponents and URLQueryItem structs.

This is my approach to solving your problem.

First I added an enum to avoid having hardcoded strings.

struct Request {
    var page: Int
    var name: String?
    var favoriteName: String?
    var favoriteId: Int?
}

enum RequestValues: String {
    case page
    case name
    case favoriteName
    case favoriteId
}

Then I made this helper function to return the non nil vars from the Request instance as an array of URLQueryItem.

func createQuery(request: Request) -> [URLQueryItem] {
    
    var queryItems: [URLQueryItem] = []
    
    queryItems.append(URLQueryItem(name: RequestValues.page.rawValue, value: "\(request.page)"))
    
    if let name = request.name {
        queryItems.append(URLQueryItem(name: RequestValues.name.rawValue, value: name))
    }
    
    if let favoriteName = request.favoriteName {
        queryItems.append(URLQueryItem(name: RequestValues.favoriteName.rawValue, value: favoriteName))
    }
    
    if let favoriteId = request.favoriteId {
        queryItems.append(URLQueryItem(name: RequestValues.favoriteId.rawValue, value: "\(favoriteId)"))
    }
    
    return queryItems
}

Then you can get the query string like this:

let queryString = queryItems.compactMap({ element -> String in
    guard let value = element.value else {
        return ""
    }
    let queryElement = "\(element.name)=\(value)"
    return queryElement
})

this will give you the expected result in your question.

page=20&name=&favoriteName=&favoriteId=25

But you should use the URLComponents struct to build your url as such.

func buildURL() -> URL? {
    
    var urlComponents = URLComponents()
    urlComponents.scheme = "https"
    urlComponents.host = "google.com"
    urlComponents.queryItems = queryItems
    urlComponents.path = "/api/example"
    urlComponents.url

    guard let url = urlComponents.url else {
        print("Could not build url")
        return nil
    }
    
    return url
}

This would will give you the url with the query.
It would look like this :

https://google.com/api/example?page=5&name=nil&favoriteName=Hello&favoriteId=9

C# Is it safe to concatenate constant strings to form a SQL Query?

While this should not present any security problem as presented. There should not be any possibility for SQL injection since it does not involve any user input.

I would still argue for using parametrized queries whenever possible, because code change. There is a risk some future developer modifies the query to add a user injected parameter, or copies the example for some other purpose that does present a SQL injection vulnerability. Using parametrized queries everywhere would simplify your code guidelines and review.

But as with everything related to security, it does depend on your specific application, threat model and other factors that only you can determine.

---

Related Topics