0

I have a String SELECT *FROM USERS WHERE ID = '@userid@' AND ROLE = '@role@' Now i have replace any string between @...@ , with a actual value .

Expected output SELECT *FROM USERS WHERE ID = '4' AND ROLE = 'Admin'

This replace will happen from a method , i have written this logic

public String replaceQueryKeyWithValueFromKeyValues(String query, int reportId) {
    try {
        REPMReportDao repmReportDao = new REPMReportDao();
        int Start = 0;
        int end;
        if (query.contains("@")) {
            boolean specialSymbolFound = false;
            for (int i = 0; i < query.length(); i++) {
                if (query.charAt(i) == '@') {
                    if (!specialSymbolFound) {
                        Start = i + 1;
                        specialSymbolFound = true;
                    } else {
                        specialSymbolFound = false;
                        end = i;
                        query = query.replace(query.substring(Start - 1, end + 1), repmReportDao.getReportManagerKeyValue(query.substring(Start - 1, end + 1).replaceAll("@", ""), reportId));

                    }
                }
            }
            return query;
        } else {
            return query;
        }

    } catch (Exception e) {
        logger.log(Priority.ERROR, e.getMessage());
        return e.getMessage();
    }
}

It works fine , but in the case if a single '@' symbol exist instead of start and end it will fail. Like :

SELECT  *FROM USERS WHERE emailid = 'xyz@gmail.com' AND ROLE = '@role@'

Here it should replace the only role '@role@' and should left email as it is.

Expected Output => SELECT *FROM USERS WHERE emailid = 'xyz@gmail.com' AND ROLE = 'Admin'

hc_dev
  • 8,389
  • 1
  • 26
  • 38
Chanky Mallick
  • 569
  • 8
  • 27
  • 1
    Please read about SQL injection and don't use String replacements to insert values in your query https://stackoverflow.com/q/332365/1515052 – Simulant Feb 23 '19 at 15:14
  • 3
    This means that the logic you applied is not working. So drop this logic, which is also not safe and use the tools that are already there and tested and working, like Prepared Statements: https://docs.oracle.com/javase/tutorial/jdbc/basics/prepared.html. – forpas Feb 23 '19 at 15:20
  • @forpas Actually here the end user writes dynamic queries from browser to get reports , he can use some value in his query which exist in database , so that value between @..@ need to be replaced by database value. And for SQL Injection, the query will get verify before execution to allow only select statements. – Chanky Mallick Feb 23 '19 at 15:28
  • you could use the `indexOf` method instead of searching the `@` yourself, or even better, regular expressions to search/substitute, (class `Pattern` to start with) The problem sample you gave is not easy to avoid (eventually use `@@` to represent a single `@`) – user85421 Feb 23 '19 at 15:34

2 Answers2

1

It's considered very bad practice to use string substitution to generate database queries, because you leave your code open to SQL Injection attacks. I can't tell from the small code sample you've provided, but the vast majority of large-scale Java projects use the Spring Framework, which allows you to use either JdbcTemplate or (my preference) NamedParameterJdbcTemplate. Both will allow you to substitute variables in a safe manner.

Jordan
  • 2,273
  • 9
  • 16
  • Actually here the end user writes dynamic queries from browser to get reports , he can use some value in his query which exist in database , so that value between @..@ need to be replaced by database value before execution. And for SQL Injection, the query will get strict verification before execution to allow only select statements. So even the user writes some DELETE,TRUNCATE,UPDATE or any DML statements , it will be blocked by QueryValidator mechanism we have. – Chanky Mallick Feb 23 '19 at 15:30
  • @Chanky see [this answer](https://stackoverflow.com/a/2309984) on how to use __NamedParameterJdbcTemplate__ for __user-defined SQL__ with __parameters templated__ by _prefix_ ‘:‘ and values passed as __Map__. Works analogue to your approach with only difference between e.g. ‘@role@‘ and ‘:role‘. – hc_dev Feb 23 '19 at 18:44
1

Complete example with mocked data returned by getReportManagerKeyValue:

import java.util.HashMap;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class StackOverflow54842971 {

    private static Map<String, String> map;

    public static void main(String[] args) {
        // preparing test data
        map = new HashMap<>();
        map.put("role", "Admin");
        map.put("userid", "666");

        // original query string
        String query = "SELECT * FROM USERS WHERE ID = '@userid@' AND emailid = 'xyz@gmail.com' AND ROLE = '@role@' ";

        // regular expression to match everything between '@ and @' with capture group
        // omitting single quotes
        Pattern p = Pattern.compile("'(@[^@]*@)'");
        Matcher m = p.matcher(query);
        while (m.find()) {
            // every match will be replaced with value from getReportManagerKeyValue
            query = query.replace(m.group(1), getReportManagerKeyValue(m.group(1).replaceAll("@", "")));
        }
        System.out.println(query);
    }

    // you won't need this function
    private static String getReportManagerKeyValue(String key) {
        System.out.println("getting key " + key);
        if (!map.containsKey(key)) {
            return "'null'";
        }
        return map.get(key);
    }
}

Krystian G
  • 2,842
  • 3
  • 11
  • 25