PreparedStatement kilka pytań

Question

Cześć,

Piszę w Javie, od niedawna bawię się JDBC (baza SQLite). Tak jak w temacie - mam kilka pytań o PreparedStatement.

Z tego co już wyczytałem w internecie to klejenie Stringów i wykorzystywanie do stworzenia obiektu PreparedStatement jest BARDZO MOCNO NIE ZALECANE ze względu na SQL injection.

Przykład kodu poniżej

public static int addToDictionary(String value, String table_name) {
    try {
        PreparedStatement select = connection.prepareStatement(
                "SELECT id FROM "+ table_name+" WHERE name = ?");
        select.setString(1,value);
        ResultSet result = select.executeQuery();
        return result.getInt(1);
    }catch (SQLException e){
        e.printStackTrace();
    }
    return -1;
}

Pytanie 1
Rozumiem zagrożenie w przypadku gdy metoda jest publiczna i/lub string pochodzi z pola tekstowego. Ale gdy metoda jest prywatna i nie pobieram Stringa table_name z pola tekstowego czy jest to dopuszczalne?
Pytanie 2
Jeżeli bym użył zamiast Stringa table_name enumerator z dostępem prywatnym to czy zmniejsza tp lub eliminuje  podatność mojego kodu na SQL injection?
Pytanie 3
Czy lepiej jest jednak powielić kilka razy kod który różni się tylko nazwą tabeli?
Powielanie kodu również jest złą praktyką.

Idąc dalej napotkałem problem z użyciem metody  setString klasy PreparedStatement. Mianowicie ? nie jest zastępąwany Stringiem. Widzę to po sprawdzeniu Create Statment za pomocą którego powstał Trigger w Bazie Danych. Do sprawdzenia używam aplikacji DB Browser for SQLite.

private static boolean createTriger(){
    try {
        PreparedStatement trigger = connection.prepareStatement(
            "Create TRIGGER IF NOT EXISTS afterInsertToDictionaryPlace "+
            "after insert on "+Table_Name.Dictionary_Places+" "+
            "begin "+
                "Insert into Sync "+
                "(table_id , row_id , create_data , user_action ) "+
                    "values( "+
                    "(Select id from TableList "+
                    "where name = ? "+
                    "limit 1), "+
                    "NEW.id, "+
                    "NEW.create_data, "+
                    "'Insert'); "+
                    "end;; ");
        trigger.setString(1,Table_Name.Dictionary_Places.toString());
        trigger.execute();
    } catch (SQLException e) {
        System.err.println("Bład przy tworzeniu Trigera");
        e.printStackTrace();
        return false;
    }
    return true;
}

private enum Table_Name {
    TableList,
    Dictionary_Places,
    Dictionary_GoodsAndServices,
    Category,
    Bill,
    Sync
}

Pytanie 4
Dlaczego trigger.setString w kodzie powyżej nie zamienia znaku zapytania na 'Dictionary_Places'? I jak to poprawić?

Mogę to zrobić inaczej ale IMO jest mniej eleganckie i mniej czytelne

PreparedStatement trigger = connection.prepareStatement(
    "Create TRIGGER IF NOT EXISTS afterInsertToDictionaryPlace "+
    "after insert on "+Table_Name.Dictionary_Places+" "+
    "begin "+
    "Insert into Sync (table_id , row_id , create_data , user_action ) "+
    "values( "+
        "(Select id from TableList "+
            "where name = \'"+Table_Name.Dictionary_Places+ "\' "+
            "limit 1), "+
        "NEW.id, "+
        "NEW.create_data, "+
        "'Insert'); "+
        "end;; ");
createTrigerAfterInsertToDictionaryPlaces.execute();

Przy prostszych zapytaniach SQL działa poprawnie. Przez prostrze zapytanie rozumiem  Select \ Insert bez podzapytań.

Answer 1

Pytanie 1

Rozumiem zagrożenie w przypadku gdy metoda jest publiczna i/lub string pochodzi z pola tekstowego. Ale gdy metoda jest prywatna i nie pobieram Stringa table_name z pola tekstowego czy jest to dopuszczalne? 

Jeśli podstawiana wartość nie będzie brana bezpośrednio od użytkownika to wydaje mi się, że nie będzie to błędem. Może tylko wystąpić coś takiego jak złamanie konwencji w kodzie. Bo np. w 50 miejscach nie będzie używany ten sposób, a nagle w 1 czy 2 jest inaczej. Niewykluczone, że spowoduje to dezinformacje. Gdy po jakimś czasie zajrzysz do tego kodu będziesz się głowił dlaczego tak jest zrobione. Mało tego, gdy będzie trzeba dopisać jakaś nową funkcjonalność, ktoś wzorując się na tym kodzie, może zrobić poważny błąd.

Pytanie 2

Jeżeli bym użył zamiast Stringa table_name enumerator z dostępem prywatnym to czy zmniejsza tp lub eliminuje  podatność mojego kodu na SQL injection? 

Pytanie 3 
Czy lepiej jest jednak powielić kilka razy kod który różni się tylko nazwą tabeli? 
Powielanie kodu również jest złą praktyką.

Pytanie tylko, jak dużo będzie w kodzie identycznie wyglądających zapytań różniących się tylko nazwą tabeli. Bo np.:

SELECT * FROM table

To jest zła praktyka. I też trzeba wziąć pod uwagę, że jeśli jedno zapytanie jest wykorzystane w dwóch miejscach to gdy pojawi się potrzeba zmiany w jednej funkcjonalności to czy przypadkiem nie zrobi się szkód w tej drugiej (jest to prawdopodobnie złamanie SRP lub OCP z SOLID'a).

 

Taka jeszcze opinia na temat triggerów jaką usłyszałem. Są one poniekąd złą praktyką bo powodują, że w tle na bazie jest ukrywana jakaś logika. Czyli tzw. efekt uboczny - łamie konwencję że coś robi jedną rzecz. Robiąc np inserta, trigger dodatkowo ustawia jakąś flagę czy coś - zapomnienie o tym, może być zaskakująco przykre.