From 21e35c13938d0021aa34d10933d7cd85f8d4ada5 Mon Sep 17 00:00:00 2001
From: apriestman <apriestman@basistech.com>
Date: Wed, 11 Mar 2020 12:52:24 -0400
Subject: [PATCH] Added comments and error handling to JniDbHelper.java Removed
 incorrect calls to free jstrings

---
 bindings/java/jni/auto_db_java.cpp            |  27 --
 .../org/sleuthkit/datamodel/JniDbHelper.java  | 231 ++++++++++++++++--
 2 files changed, 211 insertions(+), 47 deletions(-)

diff --git a/bindings/java/jni/auto_db_java.cpp b/bindings/java/jni/auto_db_java.cpp
index e12f29252..8fb5b158e 100644
--- a/bindings/java/jni/auto_db_java.cpp
+++ b/bindings/java/jni/auto_db_java.cpp
@@ -213,13 +213,6 @@ TskAutoDbJava::addImageInfo(int type, TSK_OFF_T ssize, int64_t & objId, const st
         type, ssize, tzj, size, md5j, sha1j, sha256j, devIdj, collj);
     objId = (int64_t)objIdj;
 
-   // m_jniEnv->ReleaseStringUTFChars(tzj, tz_cstr);
-  //  m_jniEnv->ReleaseStringUTFChars(md5j, md5_cstr);
-  //  m_jniEnv->ReleaseStringUTFChars(sha1j, sha1_cstr);
-  ////  m_jniEnv->ReleaseStringUTFChars(sha256j, sha256_cstr);
-  //  m_jniEnv->ReleaseStringUTFChars(devIdj, devId_cstr);
-  //  m_jniEnv->ReleaseStringUTFChars(collj, coll_cstr);
-
     if (objId < 0) {
         return TSK_ERR;
     }
@@ -331,8 +324,6 @@ TskAutoDbJava::addPoolVolumeInfo(const TSK_POOL_VOLUME_INFO* pool_vol,
         descj, pool_vol->flags);
     objId = (int64_t)objIdj;
 
-    //m_jniEnv->ReleaseStringUTFChars(descj, pool_vol->desc);
-
     if (objId < 0) {
         return TSK_ERR;
     }
@@ -360,8 +351,6 @@ TskAutoDbJava::addVolumeInfo(const TSK_VS_PART_INFO* vs_part,
         descj, vs_part->flags);
     objId = (int64_t)objIdj;
 
-    //m_jniEnv->ReleaseStringUTFChars(descj, vs_part->desc);
-
     if (objId < 0) {
         return TSK_ERR;
     }
@@ -640,10 +629,6 @@ TskAutoDbJava::addFile(TSK_FS_FILE* fs_file,
         pathj, extj);
     objId = (int64_t)objIdj;
 
-   // m_jniEnv->ReleaseStringUTFChars(namej, name);
-   // m_jniEnv->ReleaseStringUTFChars(pathj, escaped_path);
-   // m_jniEnv->ReleaseStringUTFChars(extj, extension);
-
     if (objId < 0) {
         return TSK_ERR;
     }
@@ -688,9 +673,6 @@ TskAutoDbJava::addFile(TSK_FS_FILE* fs_file,
             pathj, slackExtj);
         int64_t slackObjId = (int64_t)objIdj;
 
-      //  m_jniEnv->ReleaseStringUTFChars(slackNamej, name);
-      //  m_jniEnv->ReleaseStringUTFChars(slackExtj, extension);
-
         if (slackObjId < 0) {
             return TSK_ERR;
         }
@@ -810,8 +792,6 @@ TskAutoDbJava::addFileWithLayoutRange(const TSK_DB_FILES_TYPE_ENUM dbFileType, c
         parentObjId, fsObjId, dataSourceObjId, dbFileType, namej, size);
     objId = (int64_t)objIdj;
 
-  //  m_jniEnv->ReleaseStringUTFChars(namej, fileNameSs.str().c_str());
-
     if (objId < 0) {
         return TSK_ERR;
     }
@@ -887,8 +867,6 @@ TskAutoDbJava::addUnallocFsBlockFilesParent(const int64_t fsObjId, int64_t& objI
         fsObjId, namej);
     objId = (int64_t)objIdj;
 
-   // m_jniEnv->ReleaseStringUTFChars(namej, unallocDirName);
-
     if (objId < 0) {
         return TSK_ERR;
     }
@@ -1043,9 +1021,6 @@ TskAutoDbJava::findParObjId(const TSK_FS_FILE* fs_file, const char* parentPath,
         fs_file->name->par_addr, fsObjId, jpath, jname);
     int64_t objId = (int64_t)objIdj;
 
-  //  m_jniEnv->ReleaseStringUTFChars(jpath, parent_path);
-   // m_jniEnv->ReleaseStringUTFChars(jname, parent_name);
-
     if (objId < 0) {
         return -1;
     }
@@ -1073,8 +1048,6 @@ TskAutoDbJava::addUnallocatedPoolVolume(int vol_index, int64_t parObjId, int64_t
         descj, 0);
     objId = (int64_t)objIdj;
 
-   // m_jniEnv->ReleaseStringUTFChars(descj, desc);
-
     if (objId < 0) {
         return TSK_ERR;
     }
diff --git a/bindings/java/src/org/sleuthkit/datamodel/JniDbHelper.java b/bindings/java/src/org/sleuthkit/datamodel/JniDbHelper.java
index 2422ad7dd..23317a442 100644
--- a/bindings/java/src/org/sleuthkit/datamodel/JniDbHelper.java
+++ b/bindings/java/src/org/sleuthkit/datamodel/JniDbHelper.java
@@ -1,43 +1,94 @@
 /*
- * To change this license header, choose License Headers in Project Properties.
- * To change this template file, choose Tools | Templates
- * and open the template in the editor.
+ * Autopsy Forensic Browser
+ *
+ * Copyright 2020 Basis Technology Corp.
+ * Contact: carrier <at> sleuthkit <dot> org
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * 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 under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
  */
 package org.sleuthkit.datamodel;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import org.sleuthkit.datamodel.SleuthkitCase.CaseDbTransaction;
 
 /**
- *
+ * This is a utility class to allow the native C code to write to the 
+ * case database. All callbacks from the native code should come through this class.
+ * Any changes to the method signatures in this class will require changes to the 
+ * native code.
  */
 class JniDbHelper {
 	
-	private SleuthkitCase caseDb;
-	private CaseDbTransaction trans;
+	private static final Logger logger = Logger.getLogger(JniDbHelper.class.getName());
+	
+	private final SleuthkitCase caseDb;
+	private CaseDbTransaction trans = null;
 	
-	private Map<Long, Long> fsIdToRootDir = new HashMap<>();
+	private final Map<Long, Long> fsIdToRootDir = new HashMap<>();
 	
 	JniDbHelper(SleuthkitCase caseDb) {
 		this.caseDb = caseDb;
 		trans = null;
 	}
 	
+	/**
+	 * Start the add image transaction
+	 * 
+	 * @throws TskCoreException 
+	 */
 	void beginTransaction() throws TskCoreException {
 		trans = caseDb.beginTransaction();
 	}
 	
+	/**
+	 * Commit the add image transaction
+	 * 
+	 * @throws TskCoreException 
+	 */
 	void commitTransaction() throws TskCoreException {
 		trans.commit();
 		trans = null;
 	}
 	
+	/**
+	 * Revert the add image transaction
+	 * 
+	 * @throws TskCoreException 
+	 */
 	void revertTransaction() throws TskCoreException {
 		trans.rollback();
 		trans = null;
 	}		
 	
+	/**
+	 * Add a new image to the database.
+	 * Intended to be called from the native code during the add image process.
+	 * 
+	 * @param type
+	 * @param ssize
+	 * @param timezone
+	 * @param size
+	 * @param md5
+	 * @param sha1
+	 * @param sha256
+	 * @param deviceId
+	 * @param collectionDetails
+	 * 
+	 * @return The object ID of the new image or -1 if an error occurred
+	 */
 	long addImageInfo(int type, long ssize, String timezone, 
 			long size, String md5, String sha1, String sha256, String deviceId, 
 			String collectionDetails) {
@@ -45,52 +96,113 @@ long addImageInfo(int type, long ssize, String timezone,
 			return caseDb.addImageJNI(TskData.TSK_IMG_TYPE_ENUM.valueOf(type), ssize, size,
 					timezone, md5, sha1, sha256, deviceId, trans);
 		} catch (TskCoreException ex) {
-			ex.printStackTrace();
+			logger.log(Level.SEVERE, "Error adding image to the database", ex);
 			return -1;
 		}
 	}
 	
+	/**
+	 * Add an image name to the database. 
+	 * Intended to be called from the native code during the add image process.
+	 * 
+	 * @param objId
+	 * @param name
+	 * @param sequence
+	 * 
+	 * @return 0 if successful, -1 if not
+	 */
 	int addImageName(long objId, String name, long sequence) {
 		try {
 			caseDb.addImageNameJNI(objId, name, sequence, trans);
 			return 0;
 		} catch (TskCoreException ex) {
-			ex.printStackTrace();
+			logger.log(Level.SEVERE, "Error adding image name to the database - image obj ID: " + objId + ", image name: " + name
+					+ ", sequence: " + sequence, ex);
 			return -1;
 		}
 	}
 	
+	/**
+	 * Add a volume system to the database. 
+	 * Intended to be called from the native code during the add image process.
+	 * 
+	 * @param parentObjId
+	 * @param vsType
+	 * @param imgOffset
+	 * @param blockSize
+	 * 
+	 * @return The object ID of the new volume system or -1 if an error occurred
+	 */
 	long addVsInfo(long parentObjId, int vsType, long imgOffset, long blockSize) {
 		try {
 			VolumeSystem vs = caseDb.addVolumeSystem(parentObjId, TskData.TSK_VS_TYPE_ENUM.valueOf(vsType), imgOffset, blockSize, trans);
 			return vs.getId();
 		} catch (TskCoreException ex) {
-			ex.printStackTrace();
+			logger.log(Level.SEVERE, "Error adding volume system to the database - parent obj ID: " + parentObjId 
+					+ ", image offset: " + imgOffset, ex);
 			return -1;
 		}
 	}
 	
+	/**
+	 * Add a volume to the database. 
+	 * Intended to be called from the native code during the add image process.
+	 * 
+	 * @param parentObjId
+	 * @param addr
+	 * @param start
+	 * @param length
+	 * @param desc
+	 * @param flags
+	 * 
+	 * @return The object ID of the new volume or -1 if an error occurred
+	 */
 	long addVolume(long parentObjId, long addr, long start, long length, String desc,
 			long flags) {
 		try {
 			Volume vol = caseDb.addVolume(parentObjId, addr, start, length, desc, flags, trans);
 			return vol.getId();
 		} catch (TskCoreException ex) {
-			ex.printStackTrace();
+			logger.log(Level.SEVERE, "Error adding volume to the database - parent object ID: " + parentObjId
+				+ ", addr: " + addr, ex);
 			return -1;
 		}
 	}
 
+	/**
+	 * Add a pool to the database. 
+	 * Intended to be called from the native code during the add image process.
+	 * 
+	 * @param parentObjId
+	 * @param poolType
+	 * 
+	 * @return The object ID of the new pool or -1 if an error occurred
+	 */
 	long addPool(long parentObjId, int poolType) {
 		try {
 			Pool pool = caseDb.addPool(parentObjId, TskData.TSK_POOL_TYPE_ENUM.valueOf(poolType), trans);
 			return pool.getId();
 		} catch (TskCoreException ex) {
-			ex.printStackTrace();
+			logger.log(Level.SEVERE, "Error adding pool to the database - parent object ID: " + parentObjId, ex);
 			return -1;
 		}
 	}
 
+	/**
+	 * Add a file system to the database. 
+	 * Intended to be called from the native code during the add image process.
+	 * 
+	 * @param parentObjId
+	 * @param imgOffset
+	 * @param fsType
+	 * @param blockSize
+	 * @param blockCount
+	 * @param rootInum
+	 * @param firstInum
+	 * @param lastInum
+	 * 
+	 * @return The object ID of the new file system or -1 if an error occurred
+	 */
 	long addFileSystem(long parentObjId, long imgOffset, int fsType, long blockSize, long blockCount,
 			long rootInum, long firstInum, long lastInum) {
 		try {
@@ -98,11 +210,42 @@ long addFileSystem(long parentObjId, long imgOffset, int fsType, long blockSize,
 					rootInum, firstInum, lastInum, null, trans);
 			return fs.getId();
 		} catch (TskCoreException ex) {
-			ex.printStackTrace();
+			logger.log(Level.SEVERE, "Error adding file system to the database - parent object ID: " + parentObjId
+					+ ", offset: " + imgOffset, ex);
 			return -1;
 		}
 	}
 
+	/**
+	 * Add a file to the database. 
+	 * Intended to be called from the native code during the add image process.
+	 * 
+	 * @param parentObjId
+	 * @param fsObjId
+	 * @param dataSourceObjId
+	 * @param fsType
+	 * @param attrType
+	 * @param attrId
+	 * @param name
+	 * @param metaAddr
+	 * @param metaSeq
+	 * @param dirType
+	 * @param metaType
+	 * @param dirFlags
+	 * @param metaFlags
+	 * @param size
+	 * @param crtime
+	 * @param ctime
+	 * @param atime
+	 * @param mtime
+	 * @param meta_mode
+	 * @param gid
+	 * @param uid
+	 * @param escaped_path
+	 * @param extension
+	 * 
+	 * @return The object ID of the new file or -1 if an error occurred
+	 */
 	long addFile(long parentObjId, 
         long fsObjId, long dataSourceObjId,
         int fsType,
@@ -111,7 +254,7 @@ long addFile(long parentObjId,
         int dirType, int metaType, int dirFlags, int metaFlags,
         long size,
         long crtime, long ctime, long atime, long mtime,
-        int meta_mode, int gid, int uid, /// md5TextPtr, known,
+        int meta_mode, int gid, int uid,
         String escaped_path, String extension) {
 		try {
 			long objId = caseDb.addFileSystemFileJNI(parentObjId, 
@@ -127,16 +270,31 @@ long addFile(long parentObjId,
 				escaped_path, extension, 
 				false, trans);
 			
+			// If we're adding the root directory for the file system, cache it
 			if (parentObjId == fsObjId) {
 				fsIdToRootDir.put(fsObjId, objId);
 			}
 			return objId;
 		} catch (TskCoreException ex) {
-			ex.printStackTrace();
+			logger.log(Level.SEVERE, "Error adding file to the database - parent object ID: " + parentObjId
+					+ ", file system object ID: " + fsObjId + ", name: " + name, ex);
 			return -1;
 		}
 	}
 	
+	/**
+	 * Add a layout file to the database. 
+	 * Intended to be called from the native code during the add image process.
+	 * 
+	 * @param parentObjId
+	 * @param fsObjId
+	 * @param dataSourceObjId
+	 * @param fileType
+	 * @param name
+	 * @param size
+	 * 
+	 * @return The object ID of the new file or -1 if an error occurred
+	 */
 	long addLayoutFile(long parentObjId, 
         long fsObjId, long dataSourceObjId,
         int fileType,
@@ -165,40 +323,73 @@ long addLayoutFile(long parentObjId,
 				true, trans);
 			return objId;
 		} catch (TskCoreException ex) {
-			ex.printStackTrace();
+			logger.log(Level.SEVERE, "Error adding layout file to the database - parent object ID: " + parentObjId
+					+ ", file system object ID: " + fsObjId + ", name: " + name, ex);
 			return -1;
 		}
 	}	
 	
+	/**
+	 * Add a layout file range to the database. 
+	 * Intended to be called from the native code during the add image process.
+	 * 
+	 * @param objId
+	 * @param byteStart
+	 * @param byteLen
+	 * @param seq
+	 * 
+	 * @return 0 if successful, -1 if not
+	 */
 	long addLayoutFileRange(long objId, long byteStart, long byteLen, long seq) {
 		try {
 			caseDb.addLayoutFileRangeJNI(objId, byteStart, byteLen, seq, trans);
 			return 0;
 		} catch (TskCoreException ex) {
-			ex.printStackTrace();
+			logger.log(Level.SEVERE, "Error adding layout file range to the database - layout file ID: " + objId 
+				+ ", byte start: " + byteStart, ex);
 			return -1;
 		}
 	}
 	
+	/**
+	 * Look up the parent of a file based on metadata address and name/path.
+	 * Intended to be called from the native code during the add image process.
+	 * 
+	 * @param metaAddr
+	 * @param fsObjId
+	 * @param path
+	 * @param name
+	 * 
+	 * @return The object ID of the parent or -1 if not found
+	 */
 	long findParentObjId(long metaAddr, long fsObjId, String path, String name) {
 		try {
 			return caseDb.findParentObjIdJNI(metaAddr, fsObjId, path, name, trans);
 		} catch (TskCoreException ex) {
-			ex.printStackTrace();
+			logger.log(Level.WARNING, "Error looking up parent with meta addr: " + metaAddr + " and name " + name, ex);
 			return -1;
 		}
 	}
 	
+	/**
+	 * Add a virtual directory to hold unallocated file system blocks.
+	 * Intended to be called from the native code during the add image process.
+	 * 
+	 * @param fsObjId
+	 * @param name
+	 * 
+	 * @return The object ID of the new virtual directory or -1 if an error occurred
+	 */
 	long addUnallocFsBlockFilesParent(long fsObjId, String name) {
 		try {
 			if (! fsIdToRootDir.containsKey(fsObjId)) {
-				System.out.println("Argh no fs id...");
+				logger.log(Level.SEVERE, "Error - root directory for file system ID {0} not found", fsObjId);
 				return -1;
 			}
 			VirtualDirectory dir = caseDb.addVirtualDirectory(fsIdToRootDir.get(fsObjId), name, trans);
 			return dir.getId();
 		} catch (TskCoreException ex) {
-			ex.printStackTrace();
+			logger.log(Level.SEVERE, "Error creating virtual directory " + name + " under file system ID " + fsObjId, ex);
 			return -1;
 		}
 	}
-- 
GitLab