Merged heads
authorAsier Martirena <asier.martirena@openbravo.com>
Tue, 07 Nov 2017 13:13:14 +0100
changeset 32419 7e461095eb28
parent 32418 202b75e61a10 (current diff)
parent 32413 46910f47f213 (diff)
child 32420 ad1e5c216840
Merged heads
--- a/modules/org.openbravo.client.application/src/org/openbravo/client/application/window/StandardWindowComponent.java	Thu May 04 10:40:08 2017 +0200
+++ b/modules/org.openbravo.client.application/src/org/openbravo/client/application/window/StandardWindowComponent.java	Tue Nov 07 13:13:14 2017 +0100
@@ -97,7 +97,6 @@
 
   public String generate() {
     final String jsCode = super.generate();
-    // System.err.println(jsCode);
     return jsCode;
   }
 
--- a/modules/org.openbravo.service.datasource/src/org/openbravo/service/datasource/DataSourceServlet.java	Thu May 04 10:40:08 2017 +0200
+++ b/modules/org.openbravo.service.datasource/src/org/openbravo/service/datasource/DataSourceServlet.java	Tue Nov 07 13:13:14 2017 +0100
@@ -180,7 +180,6 @@
       log.error(e.getMessage(), e);
       writeResult(response, JsonUtils.convertExceptionToJson(e));
     } catch (final Throwable t) {
-      t.printStackTrace(System.err);
       if (SessionHandler.isSessionHandlerPresent()) {
         SessionHandler.getInstance().setDoRollback(true);
       }
--- a/modules/org.openbravo.service.datasource/src/org/openbravo/service/datasource/ReadOnlyDataSourceService.java	Thu May 04 10:40:08 2017 +0200
+++ b/modules/org.openbravo.service.datasource/src/org/openbravo/service/datasource/ReadOnlyDataSourceService.java	Tue Nov 07 13:13:14 2017 +0100
@@ -108,9 +108,6 @@
       jsonResponse.put(JsonConstants.RESPONSE_DATA, new JSONArray(jsonObjects));
       jsonResult.put(JsonConstants.RESPONSE_RESPONSE, jsonResponse);
 
-      // if (jsonObjects.size() > 0) {
-      // System.err.println(jsonObjects.get(0));
-      // }
       return jsonResult.toString();
     } catch (JSONException e) {
       throw new OBException(e);
--- a/src-core/src/org/openbravo/utils/CryptoUtility.java	Thu May 04 10:40:08 2017 +0200
+++ b/src-core/src/org/openbravo/utils/CryptoUtility.java	Tue Nov 07 13:13:14 2017 +0100
@@ -1,6 +1,6 @@
 /*
  ************************************************************************************
- * Copyright (C) 2001-2010 Openbravo S.L.U.
+ * Copyright (C) 2001-2017 Openbravo S.L.U.
  * Licensed under the Apache Software License version 2.0
  * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
  * Unless required by applicable law or agreed to  in writing,  software  distributed
@@ -12,76 +12,59 @@
 
 package org.openbravo.utils;
 
+import static org.apache.commons.codec.binary.Base64.decodeBase64;
+import static org.apache.commons.codec.binary.Base64.encodeBase64;
+
 import javax.crypto.Cipher;
 import javax.crypto.SecretKey;
 import javax.crypto.spec.SecretKeySpec;
 import javax.servlet.ServletException;
 
+/** Basic utilities to encrypt/decrypt Strings. */
 public class CryptoUtility {
-  private static Cipher s_cipher = null;
-  private static SecretKey s_key = null;
+  private static final SecretKey KEY = new SecretKeySpec(new byte[] { 100, 25, 28, -122, -26, 94,
+      -3, -72 }, "DES");
+  private static final String TRANSFORMATION = "DES/ECB/PKCS5Padding";
 
-  public CryptoUtility() {
-  }
+  /**
+   * Encrypts a String
+   * 
+   * @param value
+   *          Plain text String to be encrypted.
+   * @return Encrypted {@code value}.
+   */
+  public static String encrypt(String value) throws ServletException {
+    String clearText = value == null ? "" : value;
 
-  public static void main(String argv[]) throws Exception {
-    System.out.println("Enter encryption password:  ");
-    System.out.flush();
-    String clave = argv[0];
-    System.out.println("************* " + clave);
-    String strEnc = CryptoUtility.encrypt(clave);
-    System.out.println("ENCRYPTED TEXT: " + strEnc);
-    System.out.println("DECRYPTED TEXT: " + CryptoUtility.decrypt(strEnc));
-  }
-
-  private static void initCipher() {
     try {
-      s_key = new SecretKeySpec(new byte[] { 100, 25, 28, -122, -26, 94, -3, -72 }, "DES");
-      s_cipher = Cipher.getInstance("DES/ECB/PKCS5Padding");
+      Cipher cipher = Cipher.getInstance(TRANSFORMATION);
+      cipher.init(Cipher.ENCRYPT_MODE, KEY);
+      byte[] encString = cipher.doFinal(clearText.getBytes());
+      return new String(encodeBase64(encString), "UTF-8");
     } catch (Exception ex) {
-      ex.printStackTrace();
+      throw new ServletException("CryptoUtility.encrypt() - Can't init cipher", ex);
     }
   }
 
-  public static String encrypt(String value) throws ServletException {
-    byte encString[];
-    String clearText;
-    clearText = value;
-    if (clearText == null)
-      clearText = "";
-    if (s_cipher == null)
-      initCipher();
-    if (s_cipher == null)
-      throw new ServletException("CryptoUtility.encrypt() - Can't load cipher");
-    String result = "";
+  /**
+   * Decrypts a String
+   * 
+   * @param value
+   *          Encrypted String
+   * @return Decrypted {@code value}.
+   */
+  public static String decrypt(String value) throws ServletException {
+    if (value == null || value.length() == 0) {
+      return value;
+    }
+
     try {
-      s_cipher.init(Cipher.ENCRYPT_MODE, s_key);
-      encString = s_cipher.doFinal(clearText.getBytes());
-      result = new String(org.apache.commons.codec.binary.Base64.encodeBase64(encString), "UTF-8");
-    } catch (Exception ex) {
-      throw new ServletException("CryptoUtility.encrypt() - Can't init cipher", ex);
-    }
-    return result;
-  }
-
-  public static String decrypt(String value) throws ServletException {
-    if (value == null)
-      return null;
-    if (value.length() == 0)
-      return value;
-    if (s_cipher == null)
-      initCipher();
-    if (s_cipher == null || value == null || value.length() <= 0)
-      throw new ServletException("CryptoUtility.decrypt() - Can't load cipher");
-    byte out[];
-    byte decode[];
-    try {
-      decode = org.apache.commons.codec.binary.Base64.decodeBase64(value.getBytes("UTF-8"));
-      s_cipher.init(Cipher.DECRYPT_MODE, s_key, s_cipher.getParameters());
-      out = s_cipher.doFinal(decode);
+      byte[] decode = decodeBase64(value.getBytes("UTF-8"));
+      Cipher cipher = Cipher.getInstance(TRANSFORMATION);
+      cipher.init(Cipher.DECRYPT_MODE, KEY, cipher.getParameters());
+      return new String(cipher.doFinal(decode));
     } catch (Exception ex) {
       throw new ServletException("CryptoUtility.decrypt() - Can't init cipher", ex);
     }
-    return new String(out);
   }
 }
--- a/src-test/src/org/openbravo/test/AllAntTaskTests.java	Thu May 04 10:40:08 2017 +0200
+++ b/src-test/src/org/openbravo/test/AllAntTaskTests.java	Tue Nov 07 13:13:14 2017 +0100
@@ -93,6 +93,7 @@
 import org.openbravo.test.services.ServicesTest;
 import org.openbravo.test.services.ServicesTest2;
 import org.openbravo.test.services.ServicesTest3;
+import org.openbravo.test.system.CryptoUtilities;
 import org.openbravo.test.system.ErrorTextParserTest;
 import org.openbravo.test.system.ImportEntrySizeTest;
 import org.openbravo.test.system.Issue29934Test;
@@ -181,6 +182,7 @@
     TestInfrastructure.class, //
     Issue29934Test.class, //
     ImportEntrySizeTest.class, //
+    CryptoUtilities.class, //
 
     // xml
     ClientExportImportTest.class, //
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src-test/src/org/openbravo/test/system/CryptoUtilities.java	Tue Nov 07 13:13:14 2017 +0100
@@ -0,0 +1,87 @@
+/*
+ *************************************************************************
+ * The contents of this file are subject to the Openbravo  Public  License
+ * Version  1.1  (the  "License"),  being   the  Mozilla   Public  License
+ * Version 1.1  with a permitted attribution clause; you may not  use this
+ * file except in compliance with the License. You  may  obtain  a copy of
+ * the License at http://www.openbravo.com/legal/license.html 
+ * Software distributed under the License  is  distributed  on  an "AS IS"
+ * basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the
+ * License for the specific  language  governing  rights  and  limitations
+ * under the License. 
+ * The Original Code is Openbravo ERP. 
+ * The Initial Developer of the Original Code is Openbravo SLU
+ * All portions are Copyright (C) 2017 Openbravo SLU
+ * All Rights Reserved. 
+ * Contributor(s):  ______________________________________.
+ ************************************************************************
+ */
+
+package org.openbravo.test.system;
+
+import static org.apache.commons.lang.math.NumberUtils.DOUBLE_ZERO;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.commons.lang.RandomStringUtils;
+import org.junit.Test;
+import org.openbravo.utils.CryptoUtility;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/** Test cases for org.openbravo.utils.CryptoUtility */
+public class CryptoUtilities {
+  private static final int THREAD_NUM = 8;
+  private static final int LOOPS = 100;
+
+  private static Logger log = LoggerFactory.getLogger(CryptoUtilities.class);
+
+  /** Covers concurrent utilization of encrypt/decrypt methods. See issue #36909 */
+  @Test
+  public void shouldWorkConcurrently() throws InterruptedException, ExecutionException {
+    ExecutorService executor = Executors.newFixedThreadPool(THREAD_NUM);
+
+    List<Callable<List<String>>> tasks = new ArrayList<>();
+
+    for (int i = 0; i < THREAD_NUM; i++) {
+      tasks.add(new Callable<List<String>>() {
+        @Override
+        public List<String> call() throws Exception {
+          List<String> msg = new ArrayList<>(LOOPS);
+          for (int j = 0; j < LOOPS; j++) {
+            try {
+              String raw = RandomStringUtils.randomAlphanumeric(j);
+              String encrypted = CryptoUtility.encrypt(raw);
+              String decrypted = CryptoUtility.decrypt(encrypted);
+              if (!decrypted.equals(raw)) {
+                msg.add("Failed to decrypt");
+              }
+            } catch (Exception e) {
+              msg.add(e.getMessage());
+            }
+          }
+          return msg;
+        }
+      });
+    }
+
+    List<Future<List<String>>> execs = executor.invokeAll(tasks, 5, TimeUnit.MINUTES);
+    double totalErrors = 0;
+    for (Future<List<String>> exec : execs) {
+      for (String msg : exec.get()) {
+        log.error(msg);
+        totalErrors += 1;
+      }
+    }
+    assertThat("Error ratio", totalErrors / (THREAD_NUM * LOOPS), is(DOUBLE_ZERO));
+  }
+}
--- a/src/org/openbravo/dal/security/EntityAccessChecker.java	Thu May 04 10:40:08 2017 +0200
+++ b/src/org/openbravo/dal/security/EntityAccessChecker.java	Tue Nov 07 13:13:14 2017 +0100
@@ -302,8 +302,7 @@
   }
 
   /**
-   * Dumps the readable, writable, derived readable entities to the System.err outputstream. For
-   * debugging purposes.
+   * Dumps the readable, writable, derived readable entities. For debugging purposes.
    */
   public void dump() {
     log.info("");
--- a/src/org/openbravo/service/db/DalBaseProcess.java	Thu May 04 10:40:08 2017 +0200
+++ b/src/org/openbravo/service/db/DalBaseProcess.java	Tue Nov 07 13:13:14 2017 +0100
@@ -11,7 +11,7 @@
  * under the License. 
  * The Original Code is Openbravo ERP. 
  * The Initial Developer of the Original Code is Openbravo SLU 
- * All portions are Copyright (C) 2009-2015 Openbravo SLU 
+ * All portions are Copyright (C) 2009-2017 Openbravo SLU 
  * All Rights Reserved. 
  * Contributor(s):  ______________________________________.
  ************************************************************************
@@ -25,6 +25,7 @@
 import org.openbravo.base.secureApp.VariablesSecureApp;
 import org.openbravo.client.kernel.RequestContext;
 import org.openbravo.dal.core.OBContext;
+import org.openbravo.dal.core.SessionHandler;
 import org.openbravo.dal.service.OBDal;
 import org.openbravo.scheduling.Process;
 import org.openbravo.scheduling.ProcessBundle;
@@ -111,6 +112,12 @@
         }
       }
 
+      // if the default connection should be closed after process execution, then close also other
+      // opened connections (if any)
+      if (bundle.getCloseConnection() && SessionHandler.existsOpenedSessions()) {
+        SessionHandler.getInstance().cleanUpSessions();
+      }
+
       // remove the context at the end, maybe the process scheduler
       // reuses the thread?
       OBContext.setOBContext(currentOBContext);
--- a/src/org/openbravo/service/importprocess/ImportEntryProcessor.java	Thu May 04 10:40:08 2017 +0200
+++ b/src/org/openbravo/service/importprocess/ImportEntryProcessor.java	Tue Nov 07 13:13:14 2017 +0100
@@ -113,7 +113,7 @@
 
   // multiple threads access this map, its access is handled through
   // synchronized methods
-  private Map<String, ImportEntryProcessRunnable> runnables = new HashMap<String, ImportEntryProcessRunnable>();
+  private Map<String, ImportEntryProcessRunnable> runnables = new HashMap<>();
 
   @Inject
   private ImportEntryManager importEntryManager;
@@ -246,8 +246,10 @@
    * <li>OBContexts are temporary cached in a {@link WeakHashMap}</li>
    * <li>the process checks the {@link ImportEntry} status just before it is processed, it also
    * prevents the same {@link ImportEntry} to be processed twice by one thread</li>
-   * <li>each {@link ImportEntry} is processed in its own connection and transaction. Note the
-   * process here does not commit a transaction, the implementing subclass must do that.</li>
+   * <li>each {@link ImportEntry} is processed in its own connection and transaction. Note that the
+   * class delegates into the implementing subclass the ability of handling the commit/rollback of
+   * the transaction. But in order to prevent possible connection leaks, this class closes all the
+   * opened connections (if any) before ending.</li>
    * <li>the process sets admin mode, before calling the subclass</li>
    * <li>an error which ends up in the main loop here is stored in the {@link ImportEntry} in the
    * errorInfo property</li>
@@ -257,8 +259,8 @@
    * @author mtaal
    *
    */
-  public static abstract class ImportEntryProcessRunnable implements Runnable {
-    private Queue<QueuedEntry> importEntries = new ConcurrentLinkedQueue<QueuedEntry>();
+  public abstract static class ImportEntryProcessRunnable implements Runnable {
+    private Queue<QueuedEntry> importEntries = new ConcurrentLinkedQueue<>();
 
     private Logger logger;
 
@@ -271,7 +273,7 @@
     private String key = null;
     // use weakhashmap so that the content is automatically purged
     // when the garbagecollector runs
-    private Map<String, OBContext> cachedOBContexts = new HashMap<String, OBContext>();
+    private Map<String, OBContext> cachedOBContexts = new HashMap<>();
 
     @Inject
     @Any
@@ -406,14 +408,13 @@
             OBDal.getInstance().commitAndClose();
           }
 
-          // the import entry processEntry calls should not leave an open active session
+          // close sessions in case the import entry processEntry left them opened
           if (SessionHandler.isSessionHandlerPresent()) {
-            // change to warning if the code in the subclasses really works correctly
-            logger
-                .warn("Session handler present after processing import entry, this indicates that the processing code "
-                    + "does not correctly clean/close the session after its last actions. This should be fixed.");
             OBDal.getInstance().commitAndClose();
           }
+          if (SessionHandler.existsOpenedSessions()) {
+            SessionHandler.getInstance().cleanUpSessions();
+          }
 
         } catch (Throwable t) {
           ImportProcessUtils.logError(logger, t);
@@ -441,7 +442,7 @@
           cleanUpThreadForNextCycle();
         }
       }
-      if (logger.isDebugEnabled() & cnt > 0) {
+      if (logger.isDebugEnabled() && cnt > 0) {
         logger.debug("Runnable: " + key + ", processed " + cnt + " import entries in " + totalT
             + " millis, " + (totalT / cnt) + " per import entry, current queue size: "
             + importEntries.size());