svn commit: r1686741 - /ofbiz/trunk/applications/product/src/org/ofbiz/product/category/CategoryServices.java

[mbrohl]

Author: mbrohl
Date: Sun Jun 21 16:32:05 2015
New Revision: 1686741

URL: http://svn.apache.org/r1686741
Log:
Applied patch for OFBIZ-6522: Potential IndexOutOfBoundsException in CategoryServices.getProductCategoryAndLimitedMembers.

If view indexes as input parameters of service getProductCategoryAndLimitedMembers get manipulated (e.g. by manipulating URL params or bookmarking an URL with those params and come back later), it can lead to an IndexOutOfBoundsException because of a lowIndex greater list size. The highIndex is checked for that, the lowIndex is not.

Thanks Martin Becker for reporting the issue and providing the patch.

Modified:
    ofbiz/trunk/applications/product/src/org/ofbiz/product/category/CategoryServices.java

Modified: ofbiz/trunk/applications/product/src/org/ofbiz/product/category/CategoryServices.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/src/org/ofbiz/product/category/CategoryServices.java?rev=1686741&r1=1686740&r2=1686741&view=diff
==============================================================================
--- ofbiz/trunk/applications/product/src/org/ofbiz/product/category/CategoryServices.java (original)
+++ ofbiz/trunk/applications/product/src/org/ofbiz/product/category/CategoryServices.java Sun Jun 21 16:32:05 2015
@@ -269,7 +269,8 @@ public class CategoryServices {
             lowIndex = 0;
             highIndex = 0;
         }
-        Boolean filterOutOfStock = false ;
+        
+        boolean filterOutOfStock = false;
         try {
             String productStoreId = (String) context.get("productStoreId");
             if (UtilValidate.isNotEmpty(productStoreId)) {
@@ -281,8 +282,10 @@ public class CategoryServices {
         } catch (GenericEntityException e) {
             Debug.logWarning(e.getMessage(), module);
         }
+
         List<GenericValue> productCategoryMembers = null;
         if (productCategory != null) {
+            EntityListIterator pli = null;
             try {
                 if (useCacheForMembers) {
                     productCategoryMembers = EntityQuery.use(delegator).from(entityName).where("productCategoryId", productCategoryId).orderBy(orderByFields).cache(true).queryList();
@@ -309,22 +312,26 @@ public class CategoryServices {
                         } catch (GeneralException e) {
                             Debug.logWarning("Problem filtering out of stock products :"+e.getMessage(), module);
                         }
-                        
                     }
                     // filter out the view allow before getting the sublist
                     if (UtilValidate.isNotEmpty(viewProductCategoryId)) {
                         productCategoryMembers = CategoryWorker.filterProductsInCategory(delegator, productCategoryMembers, viewProductCategoryId);
-                        listSize = productCategoryMembers.size();
                     }
 
                     // set the index and size
-                    listSize = productCategoryMembers.size();
-                    if (highIndex > listSize) {
-                        highIndex = listSize;
-                    }
-
-                    // get only between low and high indexes
+                    listSize = productCategoryMembers.size();        
                     if (limitView) {
+                        // limit high index to (filtered) listSize
+                        if (highIndex > listSize) {
+                            highIndex = listSize;
+                        }
+                        // if lowIndex > listSize, the input is wrong => reset to first page
+                        if (lowIndex > listSize) {
+                            viewIndex = 0;
+                            lowIndex = 1;
+                            highIndex = Math.min(viewSize, highIndex);
+                        }
+                        // get only between low and high indexes
                         if (UtilValidate.isNotEmpty(productCategoryMembers)) {
                             productCategoryMembers = productCategoryMembers.subList(lowIndex-1, highIndex);
                         }
@@ -349,7 +356,7 @@ public class CategoryServices {
 
                     // set distinct on
                     // using list iterator
-                    EntityListIterator pli = EntityQuery.use(delegator).from(entityName).where(mainCond).orderBy(orderByFields).cursorScrollInsensitive().maxRows(highIndex).queryIterator();
+                    pli = EntityQuery.use(delegator).from(entityName).where(mainCond).orderBy(orderByFields).cursorScrollInsensitive().maxRows(highIndex).queryIterator();
 
                     // get the partial list for this page
                     if (limitView) {
@@ -372,13 +379,12 @@ public class CategoryServices {
                             }
                         } else {
                             productCategoryMembers = pli.getPartialList(lowIndex, viewSize);
-
                             listSize = pli.getResultsSizeAfterPartialList();
                         }
                     } else {
                         productCategoryMembers = pli.getCompleteList();
                         if (UtilValidate.isNotEmpty(viewProductCategoryId)) {
-                            // fiter out the view allow
+                            // filter out the view allow
                             productCategoryMembers = CategoryWorker.filterProductsInCategory(delegator, productCategoryMembers, viewProductCategoryId);
                         }
 
@@ -386,6 +392,7 @@ public class CategoryServices {
                         lowIndex = 1;
                         highIndex = listSize;
                     }
+
                     // filter out of stock products
                     if (filterOutOfStock) {
                         try {
@@ -395,6 +402,7 @@ public class CategoryServices {
                             Debug.logWarning("Problem filtering out of stock products :"+e.getMessage(), module);
                         }
                     }
+
                     // null safety
                     if (productCategoryMembers == null) {
                         productCategoryMembers = new LinkedList<GenericValue>();
@@ -403,13 +411,20 @@ public class CategoryServices {
                     if (highIndex > listSize) {
                         highIndex = listSize;
                     }
-
-                    // close the list iterator
-                    pli.close();
                 }
             } catch (GenericEntityException e) {
                 Debug.logError(e, module);
             }
+            finally {
+                // close the list iterator, if used
+                if (pli != null) {
+                    try {
+                        pli.close();
+                    } catch (GenericEntityException e) {
+                        Debug.logError(e, module);
+                    }
+                }
+            }
         }
 
         Map<String, Object> result = new HashMap<String, Object>();