Code refactoring

k-tamura · Oct 26, 2017 · 5b4952d · 5b4952d
1 parent f7d8b02
commit 5b4952d
Show file tree

Hide file tree

Showing 23 changed files with 163 additions and 198 deletions.
diff --git a/.gitignore b/.gitignore
@@ -28,3 +28,5 @@ nbdist/
 /logs/
 /work/
 /uploadFiles/
+/.scannerwork/
+
diff --git a/src/main/java/org/t246osslab/easybuggy4sb/controller/AbstractController.java b/src/main/java/org/t246osslab/easybuggy4sb/controller/AbstractController.java
@@ -2,6 +2,7 @@
 
 import java.util.Locale;
 
+import org.owasp.esapi.ESAPI;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -36,4 +37,26 @@ protected void setViewAndCommonObjects(ModelAndView mav, Locale locale, String v
             log.warn("viewName is null");
         }
     }
+
+    /**
+     * Encode data for use in HTML using HTML entity encoding
+     * Note that this method just call <code>ESAPI.encoder().encodeForHTML(String)</code>.
+     *
+     * @param input the text to encode for HTML
+     * @return input encoded for HTML
+     */
+    protected String encodeForHTML(String input) {
+        return ESAPI.encoder().encodeForHTML(input);
+    }
+
+    /**
+     * Encode data for use in LDAP queries.
+     * Note that this method just call <code>ESAPI.encoder().encodeForLDAP((String)</code>.
+     *
+     * @param input the text to encode for LDAP
+     * @return input encoded for use in LDAP
+     */
+    protected String encodeForLDAP(String input) {
+        return ESAPI.encoder().encodeForLDAP(input);
+    }
 }
diff --git a/src/main/java/org/t246osslab/easybuggy4sb/controller/DefaultLoginController.java b/src/main/java/org/t246osslab/easybuggy4sb/controller/DefaultLoginController.java
@@ -73,17 +73,7 @@ public ModelAndView doPost(ModelAndView mav, HttpServletRequest req, HttpServlet
             res.sendRedirect("/login");
         } else if (authUser(userid, password)) {
             /* if authentication succeeded, then reset account lock */
-            User admin = userLoginHistory.get(userid);
-            if (admin == null) {
-                User newAdmin = new User();
-                newAdmin.setUserId(userid);
-                admin = userLoginHistory.putIfAbsent(userid, newAdmin);
-                if (admin == null) {
-                    admin = newAdmin;
-                }
-            }
-            admin.setLoginFailedCount(0);
-            admin.setLastLoginFailedTime(null);
+            resetAccountLock(userid);
 
             session.setAttribute("authNMsg", "authenticated");
             session.setAttribute("userid", userid);
@@ -97,25 +87,39 @@ public ModelAndView doPost(ModelAndView mav, HttpServletRequest req, HttpServlet
             }
         } else {
             /* account lock count +1 */
-            if (userid != null) {
-                User admin = userLoginHistory.get(userid);
-                if (admin == null) {
-                    User newAdmin = new User();
-                    newAdmin.setUserId(userid);
-                    admin = userLoginHistory.putIfAbsent(userid, newAdmin);
-                    if (admin == null) {
-                        admin = newAdmin;
-                    }
-                }
-                admin.setLoginFailedCount(admin.getLoginFailedCount() + 1);
-                admin.setLastLoginFailedTime(new Date());
-            }
+            incrementAccountLockNum(userid);
+
             session.setAttribute("authNMsg", "msg.authentication.fail");
             return doGet(mav, req, res, locale);
         }
         return null;
     }
 
+    protected void incrementAccountLockNum(String userid) {
+        User admin = getUser(userid);
+        admin.setLoginFailedCount(admin.getLoginFailedCount() + 1);
+        admin.setLastLoginFailedTime(new Date());
+    }
+
+    protected void resetAccountLock(String userid) {
+        User admin = getUser(userid);
+        admin.setLoginFailedCount(0);
+        admin.setLastLoginFailedTime(null);
+    }
+
+    protected User getUser(String userid) {
+        User admin = userLoginHistory.get(userid);
+        if (admin == null) {
+            User newAdmin = new User();
+            newAdmin.setUserId(userid);
+            admin = userLoginHistory.putIfAbsent(userid, newAdmin);
+            if (admin == null) {
+                admin = newAdmin;
+            }
+        }
+        return admin;
+    }
+
     protected boolean isAccountLocked(String userid) {
         if (userid == null) {
             return false;

diff --git a/src/main/java/org/t246osslab/easybuggy4sb/controller/IndexController.java b/src/main/java/org/t246osslab/easybuggy4sb/controller/IndexController.java
@@ -10,15 +10,16 @@
 
 @Controller
 public class IndexController extends AbstractController {
-	boolean isFirstLoad = true;
+
+	private boolean isFirstLoad = true;
 
     @RequestMapping(value = "/")
     public ModelAndView init(HttpSession ses, ModelAndView mav, Locale locale) {
         ses.removeAttribute("dlpinit");
         setViewAndCommonObjects(mav, locale, "index");
         mav.addObject("title", "EasyBuggy Boot");
-        String permName = null;
-        String lblPerm = null;
+        String permName;
+        String lblPerm;
         if (System.getProperty("java.version").startsWith("1.7")) {
             permName = "PermGen space";
             lblPerm = msg.getMessage("label.permgen.space", null, locale);

diff --git a/src/main/java/org/t246osslab/easybuggy4sb/core/utils/MultiPartFileUtils.java b/src/main/java/org/t246osslab/easybuggy4sb/core/utils/MultiPartFileUtils.java
@@ -0,0 +1,49 @@
+package org.t246osslab.easybuggy4sb.core.utils;
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.web.multipart.MultipartFile;
+
+/**
+ * Utility class to handle multi part files.
+ */
+public final class MultiPartFileUtils {
+
+    private static final Logger log = LoggerFactory.getLogger(MultiPartFileUtils.class);
+
+    // squid:S1118: Utility classes should not have public constructors
+    private MultiPartFileUtils() {
+        throw new IllegalAccessError("Utility class");
+    }
+
+    /**
+     * Write uploaded file to the given path.
+     *
+     * @param savePath Path to save an uploaded file.
+     * @param filePart A part or form item that was received within a <code>multipart/form-data</code> POST request.
+     * @param fileName The uploaded file name.
+     */
+    public static boolean writeFile(String savePath, MultipartFile filePart, String fileName) throws IOException {
+        boolean isConverted = false;
+        try (OutputStream out = new FileOutputStream(savePath + File.separator + fileName);
+                InputStream in = filePart.getInputStream()) {
+            int read;
+            final byte[] bytes = new byte[1024];
+            while ((read = in.read(bytes)) != -1) {
+                out.write(bytes, 0, read);
+            }
+        } catch (FileNotFoundException e) {
+            // Ignore because file already exists
+            log.debug("Exception occurs: ", e);
+            isConverted = true;
+        }
+        return isConverted;
+    }
+}
diff --git a/src/main/java/org/t246osslab/easybuggy4sb/errors/AssertionErrorController.java b/src/main/java/org/t246osslab/easybuggy4sb/errors/AssertionErrorController.java
@@ -8,6 +8,6 @@ public class AssertionErrorController {
 
     @RequestMapping(value = "/asserr")
     public void process() {
-        assert 1 >= 2 : "Invalid!";
+        assert false : "Invalid!";
     }
 }
diff --git a/src/main/java/org/t246osslab/easybuggy4sb/performance/SlowRegularExpressionController.java b/src/main/java/org/t246osslab/easybuggy4sb/performance/SlowRegularExpressionController.java
@@ -18,7 +18,7 @@ public class SlowRegularExpressionController extends AbstractController {
 	@RequestMapping(value = "/slowre")
 	public ModelAndView process(@RequestParam(value = "word", required = false) String word, ModelAndView mav,
 			Locale locale) {
-		String message = null;
+		String message;
         setViewAndCommonObjects(mav, locale, "slowregex");
 		if (!StringUtils.isBlank(word)) {
 			if (isMatched(word)) {

diff --git a/src/main/java/org/t246osslab/easybuggy4sb/performance/StringPlusOperationController.java b/src/main/java/org/t246osslab/easybuggy4sb/performance/StringPlusOperationController.java
@@ -5,7 +5,6 @@
 import java.util.Locale;
 
 import org.apache.commons.lang3.math.NumberUtils;
-import org.owasp.esapi.ESAPI;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
@@ -66,8 +65,8 @@ private StringBuilder createMainContent(String[] characters, ModelAndView mav, L
             }
             html.append(msg.getMessage("label.execution.result", null, locale));
             html.append("<br /><br />");
-            // message.append(ESAPI.encoder().encodeForHTML(builder.toString()));
-            html.append(ESAPI.encoder().encodeForHTML(s));
+            // message.append(encodeForHTML(builder.toString()));
+            html.append(encodeForHTML(s));
         } else {
             html.append(msg.getMessage("msg.enter.positive.number", null, locale));
         }
@@ -78,15 +77,15 @@ private void appendCheckBox(String[] characters, Locale locale, StringBuilder me
             String label) {
         message.append("<p>" + msg.getMessage(label, null, locale) + "</p>");
         message.append("<p>");
-        for (int i = 0; i < allCharacters.length; i++) {
+        for (String allCharacter : allCharacters) {
             message.append("<input type=\"checkbox\" name=\"characters\" value=\"");
-            message.append(allCharacters[i]);
-            if (characters == null || Arrays.asList(characters).contains(allCharacters[i])) {
+            message.append(allCharacter);
+            if (characters == null || Arrays.asList(characters).contains(allCharacter)) {
                 message.append("\" checked=\"checked\">");
             } else {
                 message.append("\">");
             }
-            message.append(allCharacters[i]);
+            message.append(allCharacter);
             message.append(" ");
         }
         message.append("</p>");

diff --git a/src/main/java/org/t246osslab/easybuggy4sb/troubles/DBConnectionLeakController.java b/src/main/java/org/t246osslab/easybuggy4sb/troubles/DBConnectionLeakController.java
@@ -54,9 +54,9 @@ public ModelAndView process(ModelAndView mav, Locale locale) {
 
     private List<User> selectUsers() throws SQLException {
         List<User> users = new ArrayList<>();
-        Connection conn = null;
-        Statement stmt = null;
-        ResultSet rs = null;
+        Connection conn;
+        Statement stmt;
+        ResultSet rs;
         conn = jdbcTemplate.getDataSource().getConnection();
         stmt = conn.createStatement();
         rs = stmt.executeQuery("select id, name, phone, mail from users where ispublic = 'true'");

diff --git a/src/main/java/org/t246osslab/easybuggy4sb/troubles/EndlessWaitingController.java b/src/main/java/org/t246osslab/easybuggy4sb/troubles/EndlessWaitingController.java
@@ -60,8 +60,8 @@ public ModelAndView process(@RequestParam(value = "count", required = false) Str
     private File createBatchFile(int count, String tmpdir) {
 
         String osName = System.getProperty("os.name").toLowerCase();
-        String batFileName = null;
-        String firstLine = null;
+        String batFileName;
+        String firstLine;
         if (osName.toLowerCase().startsWith("windows")) {
             batFileName = "test.bat";
             firstLine = "@echo off";
@@ -70,15 +70,15 @@ private File createBatchFile(int count, String tmpdir) {
             firstLine = "#!/bin/sh";
         }
 
-        File batFile = null;
+        File batFile;
         try {
             batFile = new File(tmpdir, batFileName);
         } catch (Exception e) {
             log.error("Exception occurs: ", e);
             return null;
         }
         try (FileWriter fileWriter = new FileWriter(batFile);
-                BufferedWriter buffwriter = new BufferedWriter(fileWriter);) {
+                BufferedWriter buffwriter = new BufferedWriter(fileWriter)) {
             if (!batFile.setExecutable(true)) {
                 log.debug("batFile.setExecutable(true) returns false.");
             }
@@ -112,7 +112,7 @@ private String printInputStream(InputStream is) throws IOException {
                 if (line == null) {
                     break;
                 }
-                sb.append(line + "<br>");
+                sb.append(line).append("<br>");
             }
         }
         return sb.toString();

diff --git a/src/main/java/org/t246osslab/easybuggy4sb/troubles/FileDescriptorLeakController.java b/src/main/java/org/t246osslab/easybuggy4sb/troubles/FileDescriptorLeakController.java
@@ -24,8 +24,7 @@ public class FileDescriptorLeakController extends AbstractController {
     private long count = 0;
 
     @RequestMapping(value = "/filedescriptorleak")
-    public ModelAndView process(@RequestParam(value = "pingurl", required = false) String pingURL,
-            HttpServletRequest req, ModelAndView mav, Locale locale) {
+    public ModelAndView process(HttpServletRequest req, ModelAndView mav, Locale locale) {
 
         setViewAndCommonObjects(mav, locale, "filedescriptorleak");
         try {

diff --git a/src/main/java/org/t246osslab/easybuggy4sb/troubles/JVMCrashByEAVController.java b/src/main/java/org/t246osslab/easybuggy4sb/troubles/JVMCrashByEAVController.java
@@ -14,7 +14,7 @@
 public class JVMCrashByEAVController {
 
     @RequestMapping(value = "/jvmcrasheav")
-    public void process(HttpServletRequest req, HttpServletResponse res) {
+    public void process() {
         try {
             getUnsafe().getByte(0);
         } catch (Exception e) {

diff --git a/src/main/java/org/t246osslab/easybuggy4sb/troubles/MojibakeController.java b/src/main/java/org/t246osslab/easybuggy4sb/troubles/MojibakeController.java
@@ -2,11 +2,8 @@
 
 import java.util.Locale;
 
-import javax.servlet.http.HttpServletRequest;
-
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.text.WordUtils;
-import org.owasp.esapi.ESAPI;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
@@ -18,13 +15,13 @@ public class MojibakeController extends AbstractController {
 
     @RequestMapping(value = "/mojibake")
     public ModelAndView process(@RequestParam(value = "string", required = false) String string, ModelAndView mav,
-    		HttpServletRequest req, Locale locale) {
+    		Locale locale) {
         setViewAndCommonObjects(mav, locale, "mojibake");
         if (!StringUtils.isBlank(string)) {
             // Capitalize the given string
             String capitalizedName = WordUtils.capitalize(string);
             mav.addObject("msg", msg.getMessage("label.capitalized.string", null, locale) + " : "
-                    + ESAPI.encoder().encodeForHTML(capitalizedName));
+                    + encodeForHTML(capitalizedName));
         } else {
             mav.addObject("msg", msg.getMessage("msg.enter.string", null, locale));
         }

diff --git a/src/main/java/org/t246osslab/easybuggy4sb/troubles/NetworkSocketLeakController.java b/src/main/java/org/t246osslab/easybuggy4sb/troubles/NetworkSocketLeakController.java
@@ -17,8 +17,8 @@ public class NetworkSocketLeakController extends AbstractController {
     @RequestMapping(value = "/netsocketleak")
     public ModelAndView process(ModelAndView mav, HttpServletRequest req, Locale locale) {
         setViewAndCommonObjects(mav, locale, "netsocketleak");
-        HttpURLConnection connection = null;
-        URL url = null;
+        HttpURLConnection connection;
+        URL url;
         String pingURL = req.getParameter("pingurl");
         try {
             if (pingURL == null) {

diff --git a/src/main/java/org/t246osslab/easybuggy4sb/vulnerabilities/CSRFController.java b/src/main/java/org/t246osslab/easybuggy4sb/vulnerabilities/CSRFController.java
@@ -11,7 +11,6 @@
 import javax.servlet.http.HttpSession;
 
 import org.apache.commons.lang.StringUtils;
-import org.owasp.esapi.ESAPI;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.ldap.core.LdapTemplate;
 import org.springframework.stereotype.Controller;
@@ -48,7 +47,7 @@ protected ModelAndView doPost(ModelAndView mav, HttpServletRequest req, HttpServ
 				ModificationItem item = new ModificationItem(DirContext.REPLACE_ATTRIBUTE,
 						new BasicAttribute("userPassword", password));
 				ldapTemplate.modifyAttributes(
-						"uid=" + ESAPI.encoder().encodeForLDAP(userid.trim()) + ",ou=people,dc=t246osslab,dc=org",
+						"uid=" + encodeForLDAP(userid.trim()) + ",ou=people,dc=t246osslab,dc=org",
 						new ModificationItem[] { item });
             } catch (Exception e) {
                 log.error("Exception occurs: ", e);

diff --git a/src/main/java/org/t246osslab/easybuggy4sb/vulnerabilities/ClickJackingController.java b/src/main/java/org/t246osslab/easybuggy4sb/vulnerabilities/ClickJackingController.java
@@ -13,7 +13,6 @@
 import javax.servlet.http.HttpSession;
 
 import org.apache.commons.lang.StringUtils;
-import org.owasp.esapi.ESAPI;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.ldap.core.LdapTemplate;
 import org.springframework.stereotype.Controller;
@@ -55,7 +54,7 @@ protected ModelAndView doPost(ModelAndView mav, HttpServletRequest req, HttpServ
 				ModificationItem item = new ModificationItem(DirContext.REPLACE_ATTRIBUTE,
 						new BasicAttribute("mail", mail));
 				ldapTemplate.modifyAttributes(
-						"uid=" + ESAPI.encoder().encodeForLDAP(userid.trim()) + ",ou=people,dc=t246osslab,dc=org",
+						"uid=" + encodeForLDAP(userid.trim()) + ",ou=people,dc=t246osslab,dc=org",
 						new ModificationItem[] { item });
 				mav.addObject("mail", mail);
-Original file line number
+Diff line change
@@ Expand Up / @@ -28,3 +28,5 @@ nbdist/ @@
     /logs/
     /work/
     /uploadFiles/
+    /.scannerwork/