Menu

#89 Pm select without limits

Fixed
High
Defect
2018-10-09
2018-10-05
nyj3c
No

PrivateMessageModel has select without limits. This is a problem of security, performance and UI convenience. Using pagination would solve these problems.

And BookmarkModel has the same problems. Idk the pagination would solve this for bookmarks. perhaps it is better to simply add a limit on the number of stored bookmarks.

--- without_pm_pagination/src/main/config/database/generic/generic_queries.sql
+++ pm_pagination/src/main/config/database/generic/generic_queries.sql
@@ -571,7 +571,8 @@
 PrivateMessageModel.baseListing = SELECT pm.privmsgs_type, pm.privmsgs_id, pm.privmsgs_date, pm.privmsgs_subject, u.user_id, u.username \
     FROM jforum_privmsgs pm, jforum_users u \
     #FILTER# \
-    ORDER BY pm.privmsgs_date DESC
+    ORDER BY pm.privmsgs_date DESC \
+    LIMIT ?, ?

 PrivateMessageModel.inbox = WHERE privmsgs_to_userid = ? \
     AND u.user_id = pm.privmsgs_from_userid \
@@ -589,7 +590,15 @@
     FROM jforum_privmsgs p, jforum_privmsgs_text pt \
     WHERE p.privmsgs_id = pt.privmsgs_id \
     AND p.privmsgs_id = ?
+    
+PrivateMessageModel.sentTotal = SELECT COUNT(1) AS total FROM jforum_privmsgs WHERE privmsgs_from_userid = ? \
+    AND privmsgs_type = 2

+PrivateMessageModel.inboxTotal = SELECT COUNT(1) AS total FROM jforum_privmsgs WHERE privmsgs_to_userid = ? \
+    AND ( privmsgs_type = 1 \
+    OR privmsgs_type = 0 \
+    OR privmsgs_type = 5)
+
 # #################
 # UserSessionModel
 # #################
@@ -856,4 +865,3 @@
 Spam.selectAll = SELECT pattern FROM jforum_spam ORDER BY pattern ASC
 Spam.create = INSERT INTO jforum_spam (pattern) values (?)
 Spam.delete = DELETE FROM jforum_spam WHERE pattern = ?
-

--- without_pm_pagination/src/main/config/database/hsqldb/hsqldb.sql
+++ pm_pagination/src/main/config/database/hsqldb/hsqldb.sql
@@ -130,6 +130,11 @@
 # #####################
 PrivateMessagesModel.lastGeneratedPmId = SELECT MAX(privmsgs_id) FROM jforum_privmsgs

+PrivateMessageModel.baseListing = SELECT LIMIT ? ? pm.privmsgs_type, pm.privmsgs_id, pm.privmsgs_date, pm.privmsgs_subject, u.user_id, u.username \
+    FROM jforum_privmsgs pm, jforum_users u \
+    #FILTER# \
+    ORDER BY pm.privmsgs_date DESC
+
 # #############
 # SmiliesModel
 # #############

--- without_pm_pagination/src/main/config/database/oracle/oracle.sql
+++ pm_pagination/src/main/config/database/oracle/oracle.sql
@@ -226,6 +226,15 @@
 PrivateMessagesModel.addTextField = SELECT privmsgs_text from jforum_privmsgs_text WHERE privmsgs_id = ? FOR UPDATE
 PrivateMessagesModel.lastGeneratedPmId = SELECT jforum_privmsgs_seq.currval FROM dual

+PrivateMessageModel.baseListing = SELECT * FROM ( \
+    SELECT pm.privmsgs_type, pm.privmsgs_id, pm.privmsgs_date, pm.privmsgs_subject, u.user_id, u.username, \
+    ROW_NUMBER() OVER(ORDER BY privmsgs_id) - 1 LINENUM \
+    FROM jforum_privmsgs pm, jforum_users u \
+    #FILTER# \
+    ORDER BY pm.privmsgs_date DESC \
+    ) \
+    WHERE LINENUM >= ? AND LINENUM < ?
+
 # #############
 # SmiliesModel
 # #############

--- without_pm_pagination/src/main/config/database/postgresql/postgresql.sql
+++ pm_pagination/src/main/config/database/postgresql/postgresql.sql
@@ -135,6 +135,12 @@
 # #####################
 PrivateMessagesModel.lastGeneratedPmId = SELECT CURRVAL('jforum_privmsgs_seq')

+PrivateMessageModel.baseListing = SELECT pm.privmsgs_type, pm.privmsgs_id, pm.privmsgs_date, pm.privmsgs_subject, u.user_id, u.username \
+    FROM jforum_privmsgs pm, jforum_users u \
+    #FILTER# \
+    ORDER BY pm.privmsgs_date DESC \
+    OFFSET ? LIMIT ?
+
 # #############
 # SmiliesModel
 # #############

--- without_pm_pagination/src/main/config/database/sqlserver/sqlserver.sql
+++ pm_pagination/src/main/config/database/sqlserver/sqlserver.sql
@@ -167,6 +167,13 @@
 # #############
 PrivateMessagesModel.lastGeneratedPmId = SELECT IDENT_CURRENT('jforum_privmsgs') AS privmsgs_id 

+PrivateMessageModel.baseListing = SELECT * \
+    FROM ( SELECT ROW_NUMBER() OVER (ORDER BY pm.privmsgs_date DESC) - 1 AS rownumber, \
+    pm.privmsgs_type, pm.privmsgs_id, pm.privmsgs_date, pm.privmsgs_subject, u.user_id, u.username \
+    FROM jforum_privmsgs pm, jforum_users u \
+    #FILTER# \
+    WHERE rownumber >= ? and rownumber < ?
+
 # #############
 # SmiliesModel
 # #############

--- without_pm_pagination/src/main/config/urlPattern.properties
+++ pm_pagination/src/main/config/urlPattern.properties
@@ -61,7 +61,9 @@

 # Private Messages
 pm.inbox.0 = 
+pm.inbox.1 = start
 pm.sentbox.0 = 
+pm.sentbox.1 = start
 pm.send.0 = 
 pm.sendTo.1 = user_id
 pm.read.1 = id

--- without_pm_pagination/src/main/java/net/jforum/dao/PrivateMessageDAO.java
+++ pm_pagination/src/main/java/net/jforum/dao/PrivateMessageDAO.java
@@ -86,7 +86,7 @@
     * @return A <code>List</code> with all messages found. Each 
     * entry is a <code>PrivateMessage</code> entry.
     */
-   List<PrivateMessage> selectFromInbox(User user) ;
+   List<PrivateMessage> selectFromInbox(User user, int startFrom, int count) ;

    /**
     * Selects all messages from the user's sent box. 
@@ -95,7 +95,7 @@
     * @return A <code>List</code> with all messages found. Each 
     * entry is a <code>PrivateMessage</code> entry.
     */
-   List<PrivateMessage> selectFromSent(User user) ;
+   List<PrivateMessage> selectFromSent(User user, int startFrom, int count) ;

    /**
     * Gets a <code>PrivateMessage</code> by its id.
@@ -105,4 +105,20 @@
     * @return The privMsg contents
     */
    PrivateMessage selectById(PrivateMessage privMsg) ;
+   
+   /**
+    * Gets the number of messages from the user's sent box.
+    * 
+    * @param userId
+    * @return The number of messages
+    */
+   int getTotalSent(int userId) ;
+   
+   /**
+    * Gets the number of messages from the user's inbox.
+    * 
+    * @param userId
+    * @return The number of messages
+    */
+   int getTotalInbox(int userId) ;
 }

--- without_pm_pagination/src/main/java/net/jforum/dao/generic/GenericPrivateMessageDAO.java
+++ pm_pagination/src/main/java/net/jforum/dao/generic/GenericPrivateMessageDAO.java
@@ -185,7 +185,7 @@
    /**
     * @see net.jforum.dao.PrivateMessageDAO#selectFromInbox(net.jforum.entities.User)
     */
-   @Override public List<PrivateMessage> selectFromInbox(User user)
+   @Override public List<PrivateMessage> selectFromInbox(User user, int startFrom, int count)
    {
        String query = SystemGlobals.getSql("PrivateMessageModel.baseListing");
        query = query.replaceAll("#FILTER#", SystemGlobals.getSql("PrivateMessageModel.inbox"));
@@ -195,6 +195,8 @@
        try {
            pstmt = JForumExecutionContext.getConnection().prepareStatement(query);
            pstmt.setInt(1, user.getId());
+           pstmt.setInt(2, startFrom);
+           pstmt.setInt(3, count);

            List<PrivateMessage> pmList = new ArrayList<PrivateMessage>();

@@ -224,7 +226,7 @@
    /**
     * @see net.jforum.dao.PrivateMessageDAO#selectFromSent(net.jforum.entities.User)
     */
-   @Override public List<PrivateMessage> selectFromSent(User user)
+   @Override public List<PrivateMessage> selectFromSent(User user, int startFrom, int count)
    {
        String query = SystemGlobals.getSql("PrivateMessageModel.baseListing");
        query = query.replaceAll("#FILTER#", SystemGlobals.getSql("PrivateMessageModel.sent"));
@@ -234,6 +236,8 @@
        try {
            pstmt = JForumExecutionContext.getConnection().prepareStatement(query);
            pstmt.setInt(1, user.getId());
+           pstmt.setInt(2, startFrom);
+           pstmt.setInt(3, count);

            List<PrivateMessage> pmList = new ArrayList<PrivateMessage>();

@@ -344,6 +348,64 @@
        }
        finally {
            DbUtils.close(pstmt);
+       }
+   }
+
+   /**
+    * @see net.jforum.dao.PrivateMessageDAO#getTotalSent(int)
+    */
+   @Override public int getTotalSent(int userId)
+   {
+       int total = 0;
+
+       PreparedStatement pstmt = null;
+       ResultSet rs = null;
+       try {
+           pstmt = JForumExecutionContext.getConnection().prepareStatement(
+                   SystemGlobals.getSql("PrivateMessageModel.sentTotal"));
+           pstmt.setInt(1, userId);
+
+           rs = pstmt.executeQuery();
+           if (rs.next()) {
+               total = rs.getInt("total");
+           }
+
+           return total;
+       }
+       catch (SQLException e) {
+           throw new DatabaseException(e);
+       }
+       finally {
+           DbUtils.close(rs, pstmt);
+       }
+   }
+
+   /**
+    * @see net.jforum.dao.PrivateMessageDAO#getTotalInbox(int)
+    */
+   @Override public int getTotalInbox(int userId)
+   {
+       int total = 0;
+
+       PreparedStatement pstmt = null;
+       ResultSet rs = null;
+       try {
+           pstmt = JForumExecutionContext.getConnection().prepareStatement(
+                   SystemGlobals.getSql("PrivateMessageModel.inboxTotal"));
+           pstmt.setInt(1, userId);
+
+           rs = pstmt.executeQuery();
+           if (rs.next()) {
+               total = rs.getInt("total");
+           }
+
+           return total;
+       }
+       catch (SQLException e) {
+           throw new DatabaseException(e);
+       }
+       finally {
+           DbUtils.close(rs, pstmt);
        }
    }
 }

--- without_pm_pagination/src/main/java/net/jforum/view/forum/PrivateMessageAction.java
+++ pm_pagination/src/main/java/net/jforum/view/forum/PrivateMessageAction.java
@@ -83,13 +83,23 @@
        User user = new User();
        user.setId(SessionFacade.getUserSession().getUserId());

-       List<PrivateMessage> pmList = DataAccessDriver.getInstance().newPrivateMessageDAO().selectFromInbox(user);
+       int postsPerPage = SystemGlobals.getIntValue(ConfigKeys.POSTS_PER_PAGE);
+       int start = ViewCommon.getStartPage();
+       
+       PrivateMessageDAO pmdao = DataAccessDriver.getInstance().newPrivateMessageDAO();
+       
+       int totalMessages = pmdao.getTotalInbox(user.getId());
+       
+       List<PrivateMessage> pmList = pmdao.selectFromInbox(user, start, postsPerPage);

        this.setTemplateName(TemplateKeys.PM_INBOX);
        this.context.put("inbox", true);
        this.context.put("pmList", pmList);
        this.context.put("pageTitle", I18n.getMessage("ForumBase.privateMessages")+" "+I18n.getMessage("PrivateMessage.inbox"));    
        this.putTypes();
+
+       ViewCommon.contextToPagination(start, totalMessages, postsPerPage);
+       this.context.put("postsPerPage", Integer.valueOf(postsPerPage));
    }

    public void sentbox()
@@ -102,13 +112,23 @@
        User user = new User();
        user.setId(SessionFacade.getUserSession().getUserId());

-       List<PrivateMessage> pmList = DataAccessDriver.getInstance().newPrivateMessageDAO().selectFromSent(user);
+       int postsPerPage = SystemGlobals.getIntValue(ConfigKeys.POSTS_PER_PAGE);
+       int start = ViewCommon.getStartPage();
+       
+       PrivateMessageDAO pmdao = DataAccessDriver.getInstance().newPrivateMessageDAO();
+       
+       int totalMessages = pmdao.getTotalSent(user.getId());
+       
+       List<PrivateMessage> pmList = DataAccessDriver.getInstance().newPrivateMessageDAO().selectFromSent(user, start, postsPerPage);

        this.context.put("sentbox", true);
        this.context.put("pmList", pmList);
        this.setTemplateName(TemplateKeys.PM_SENTBOX);
        this.context.put("pageTitle", I18n.getMessage("ForumBase.privateMessages")+" "+I18n.getMessage("PrivateMessage.sentbox"));
        this.putTypes();
+
+       ViewCommon.contextToPagination(start, totalMessages, postsPerPage);
+       this.context.put("postsPerPage", Integer.valueOf(postsPerPage));
    }

    private void putTypes()

--- without_pm_pagination/src/main/resources/templates/default/pm_list.htm
+++ pm_pagination/src/main/resources/templates/default/pm_list.htm
@@ -1,5 +1,7 @@
 <#include "header.htm"/>
+<#import "../macros/pagination.ftl" as pagination/>
 <script type="text/javascript" src="${JForumContext.encodeURL("/js/list/userLocalTime")}?${startupTime}"></script>
+<script type="text/javascript" src="${contextPath}/templates/${templateName}/js/pagination.js?${startupTime}"></script>
 <table cellspacing="0" cellpadding="10" width="100%" align="center">
    <tbody>
        <tr>
@@ -80,6 +82,9 @@
                                    <span class="nav">
                                     <a class="nav" href="${JForumContext.encodeURL("/forums/list")}">${I18n.getMessage("ForumListing.forumIndex")}</a>
                                     </span>
+                               </td>
+                               <td class="nav" nowrap="nowrap" align="right">
+                                   <@pagination.doPagination action/>
                                </td>
                            </tr>
                        </tbody>
2 Attachments

Related

Wiki: NewFeatures260

Discussion

  • Ulf Dittmer

    Ulf Dittmer - 2018-10-06

    Pagination is a good feature. It'll also be needed in the mobile view, but that's similar to the desktop view, so no big deal.

    What is the csrfguard-3.1-latest.jar file for? If it's different from the one available for download from SF, I'll need a source code patch.

     
  • nyj3c

    nyj3c - 2018-10-06

    What is the csrfguard-3.1-latest.jar file for? If it's different from the one available for download from SF, I'll need a source code patch.

    Сan not delete or edit this. It does not apply to this ticket.

     
  • Ulf Dittmer

    Ulf Dittmer - 2018-10-07
    • Attachments has changed:

    Diff:

    --- old
    +++ new
    @@ -1,3 +1,2 @@
    -csrfguard-3.1-latest.jar (172.1 kB; application/x-java-archive)
     pm_pagination.patch (11.6 kB; text/x-patch)
     pm_pagination.png (365.4 kB; image/png)
    
     
  • Ulf Dittmer

    Ulf Dittmer - 2018-10-07
    • assigned_to: Ulf Dittmer
     
  • Ulf Dittmer

    Ulf Dittmer - 2018-10-09
    • status: New --> Fixed
     
  • Ulf Dittmer

    Ulf Dittmer - 2018-10-09

    Checked in. Feel free to submit a pagination patch for bookmarks as well - the number of bookmarks should not be limited, though.

     

Log in to post a comment.