0

I have been working on to fix SQL injections in a very old Java + Spring MVC code base with few hundred classes at DAO layer which is currently using java.sql.PreparedStatement & java.sql.Connection .

DB Connection isolation level , DB Connection commit & Connection rollback is directly handled on Connection object using - Connection.setIsolationLevel(int isolationLevel), Connection.commit() & Connection.rollback() .

Lets say , I have a method like below ,

     public List<String> getReportName() {
            try {

Connection  connection = getConnection();
connection.setIsolationLevel(Isolation.READ_UNCOMMITTED); // Just an example
                String sql = //ActualSQLString;
                PreparedStatement  pstmt = connection.prepareStatement(sql);
                ResultSet  rs = pstmt.executeQuery();
                while (rs.next()) {
                 ......
                 ......
                }
            } catch (Exception e) {
                ...
            } finally {
                //Close connection, statement & result set
            }
            }

if I wish to introduce org.springframework.jdbc.core.JdbcTemplate in place of java.sql.PreparedStatement , there is an automatic requirement that I get rid of call connection.setTransactionIsolation(isolationLevel) , commit & rollback since JdbcTemplate works on DatSource instead of individual connection object. So I change above method as below. getJdbcTemplate() is for illustration purposes only & I can have that via @Autowired too. Also, this requirement has nothing to do with core requirement of fixing SQL injections.

@Transactional(isolation = Isolation.READ_UNCOMMITTED, readOnly = true)
public List<String> getReportName() {
            try {
                String sql = //ActualSQLString;
                return getJdbcTemplate().queryForList(sql);
            } catch (Exception e) {
                ...
            } 
            }

Scenario of commit & rollback is handled by rollbackFor attribute if that would have been happening in this above method.

Now , I am confused about when method signature looks like below i.e. Connection gets created in another DAO Class method, isolation level set there and passed on to this method ( which is in another DAO class ) ,

public List<String> getReportName(Connection  connection) {
            try {
                String sql = //ActualSQLString;
                PreparedStatement  pstmt = connection.prepareStatement(sql);
                ResultSet  rs = pstmt.executeQuery();
                while (rs.next()) {
                 ......
                 ......
                }
            } catch (Exception e) {
                ...
            } finally {
                //Close connection, statement & result set
            }
            }

There are various callers of this above method from different classes & call hierarchy is usually multiple levels i.e. Connection object was created two or three levels above & isolation set there.

e.g. Connection is created in DAOClass1.method() and passed on to above getReportName of DAOClass3 .

DAOClass1.method() -> DAOClass2.method(Conenction connection) -> DAOClass3.getReportName(Conenction connection)

Is this scenario re engineering possible by introducing @Transactional & JdbcTemplate combo ? Would I be applying @Transactional only at call initiator where Connection is created or at this method too?

I guess , this is more of a transaction propagation case but confused a bit.

My question is duplicate of below Question # 1 but need solution for my specific scenario.

Related Question 1 - Variable transaction isolation levels by request

Related Question 2 - How can I get a spring JdbcTemplate to read_uncommitted?

Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
Sabir Khan
  • 9,826
  • 7
  • 45
  • 98
  • Multiple down votes means something is seriously wrong with this question. I can edit , delete or provide information if asked for but nobody is asking for it or suggesting. Very typical. Please let me know. – Sabir Khan Oct 04 '18 at 08:49
  • Your question is confusing, unclear and too broad. – Mark Rotteveel Oct 04 '18 at 11:14
  • Thanks for the comment, tried to edit it, my question is duplicate of **Related Question 1** but scenario is a bit different. – Sabir Khan Oct 04 '18 at 11:52
  • TLDR; No code, link only - probably will be closed in no time - pitty, as you put a lot of effort into creating this. – Antoniossss Oct 04 '18 at 11:54
  • You have actually asked many different questions. All generally related to what you are trying to do, but unrelated in terms of specific problems to solve (i.e. the _too general_ comment above) I might suggest trying to save the content of all of this offline, delete this question yourself, break down your content into very specific problems you are having and ask them _one at a time_. Be sure to include expected result, what you are seeing and if possible a [mcve]. – ryyker Oct 04 '18 at 13:31
  • @MarkRotteveel : I have edited question, please let me know if this makes sense now. – Sabir Khan Oct 05 '18 at 13:40
  • 1
    @ryyker: Please see if it fits the requirement now. I have provided some code snippets to explain the scenario. – Sabir Khan Oct 05 '18 at 13:43
  • Its better. Would be even better with an even narrower scope. (i.e. 1 or two questions about a single issue, instead of many questions about multiple issues) However, I did vote to reopen. :) – ryyker Oct 05 '18 at 16:06

1 Answers1

2

I'm doubtful about the premise that dynamic isolation levels are a requirement here. When isolation levels make a difference it's not because of something about the particular data, it's the way the statements themselves are combined that is the issue. It seems unlikely that you're going to have a case where a method called with one set of data should have one isolation level, while the same method called with another set of data should go through under another isolation level.

I'm guessing the core issue with the legacy code is that there is no concept of a transactional service layer. Instead you have a hodge-podge of data access objects that are called directly by controllers, and isolation levels seem to be specified haphazardly.

Specifying transactional details like isolation level needs to be done at the service level. I would go through the controllers, separate the web-layer stuff they do from business logic, and push the business logic down into service methods that you can annotate with things like isolation level.

Once you do that you will have DAOs that can be reused in different services, where the isolation level is given in the specific service, and all the code fetching connections, catching exceptions, and closing jdbc resources can be deleted.

Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
  • Yes , you are correct that transaction details are not well organized in legacy code and your answer helped me to have better understanding. For the time being, it looks a complicated change to me and I might end up fixing only SQL injections for these cases and refrain from using `JdbcTemplate` & `@Transactional` for this particular scenario. – Sabir Khan Oct 07 '18 at 06:48