I am developing JSafebox, a portable safebox project written in Java.

A first review have been completed here.

The goal is to provide a secured environment where the user can browse encrypted files without leaking data on the drive.

The project is highly focused on security and I would appreciate any feedback that would help improve the quality of the code.

Core logic and sensitive method are located in below classes but feedback from other classes is welcome as well :)

org/ortis/jsafebox/Safe.java

/*******************************************************************************
 * Copyright 2018 Ortis (cao.ortis.org@gmail.com)
 * 
 * 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.ortis.jsafebox;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.Closeable;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.RandomAccessFile;
import java.lang.reflect.Type;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.security.Key;
import java.security.SecureRandom;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.TreeMap;
import java.util.concurrent.CancellationException;

import javax.crypto.Cipher;
import javax.crypto.SecretKey;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.SecretKeySpec;

import org.ortis.jsafebox.hash.Hasher;
import org.ortis.jsafebox.hash.SHA256;
import org.ortis.jsafebox.task.TaskProbe;

import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;

/**
 * Virtual vault where files are stored
 * 
 * @author Ortis <br>
 *         2018 Apr 26 7:29:29 PM <br>
 */
public class Safe implements Closeable
{
    public static final String VERSION = "0.2 beta";

    public static final Gson GSON = new Gson();
    private static final Type MAP_STRING_STRING_TYPE = new TypeToken<Map<String, String>>()
    {
    }.getType();

    public static final Type BYTE_ARRAY_TYPE = new TypeToken<byte []>()
    {
    }.getType();

    private final static Hasher HASHER = new SHA256();

    public static final String ENCRYPTION_LABEL = "encryption";
    public final static String ENCRYPTION_IV_LENGTH_LABEL = "iv length";
    public static final String KEY_ALGO_LABEL = "algo";
    public static final String PROTOCOL_SPEC_LABEL = "protocol description";
    public static final String PBKDF2_SALT_LABEL = "pbkdf2 salt";
    public static final int PBKDF2_ITERATION = 100000;
    public static final String PBKDF2_ITERATION_LABEL = "pbkdf2 iteration";

    public static final String PROTOCOL_SPEC = "JSafebox is using a very simple protocol so encrypted files can be easily read by another program, as long as you have the password. The encryption key is derived from the password using PBKDF2 hashing with 100000 iteration. A JSafebox file contains a SHA256 integrity hash followed by blocks: [ integrity hash | block 0 | block 1 | ... | block N ]. Each block is stored as followed: [ IV | metadata length | metadata | data length | data ] where 'IV' is the Initialization_vector of the encryption (16 bytes), 'metadata' is a JSON string and 'length' are 64 bits (8 bytes) integer. The first block 'block 0' is the 'header' and is the only block not encrypted and therefore, the only block without IV. The 'header' only have metadata ('data length' is 0) and contains text entries specified by the user and various additional entries including a protocol explanation, the type of encoding and the parameters of the encryption. The 'header's metadata is stored as JSON string and can be seen by opening the safe file with a basic text editor. The second block 'block 1' is the 'properties'. It is similar to the 'header' except that it is encrypted and have an IV. The 'properties' contains text entries specified by the user and stored in JSON. The following blocks (from 2 to N) are the encrypted files. (Full manual at https://github.com/0rtis/jsafebox)";

    private final File originalFile;

    private final SecretKey encryptionKey;
    private final int ivLength;
    private final RandomAccessFile original;

    private final File tempFile;
    private final RandomAccessFile temp;

    private final byte [] hash;

    private final Map<String, String> publicHeader;
    private final Map<String, String> privateProperties;
    private final Map<String, Block> roBlocks;
    private final Map<String, Block> blocks;
    private final Map<String, Block> tempBlocks;
    private final Map<String, Block> deletedBlocks;

    private final int bufferSize;

    private final Folder root;

    /**
     * Create an instance of {@link Safe}
     * 
     * @param file:
     *            the safe file
     * @param cipher:
     *            cipher to decrypt the data
     * @param keySpec:
     *            key specification
     * @param algoSpec:
     *            encryption specification
     * @param bufferSize:
     *            size of the <code>byte</code> buffer to be used in IO operation
     * @throws Exception
     */
    public Safe(final File file, final SecretKey key, final int bufferSize) throws Exception
    {

        this.originalFile = file.getAbsoluteFile();

        this.encryptionKey = key;

        this.bufferSize = bufferSize;

        this.original = new RandomAccessFile(file, "rw");
        this.tempFile = Files.createTempFile(null, null).toFile();
        this.temp = new RandomAccessFile(this.tempFile, "rw");

        final HashMap<String, String> publicProps = new LinkedHashMap<>();
        this.publicHeader = Collections.unmodifiableMap(publicProps);
        final HashMap<String, String> props = new LinkedHashMap<>();
        this.privateProperties = Collections.unmodifiableMap(props);
        this.blocks = new LinkedHashMap<>();
        this.roBlocks = Collections.unmodifiableMap(blocks);
        this.tempBlocks = new LinkedHashMap<>();
        this.deletedBlocks = new LinkedHashMap<>();
        this.root = new Folder(null, Folder.ROOT_NAME);

        final byte [] buffer = new byte[bufferSize];
        final byte [] outBuffer = new byte[bufferSize];

        this.original.read(buffer, 0, HASHER.getHashLength());
        this.hash = new byte[HASHER.getHashLength()];
        System.arraycopy(buffer, 0, this.hash, 0, this.hash.length);

        long length;
        int read;
        final ByteArrayOutputStream baos = new ByteArrayOutputStream(buffer.length);

        final long headerLength = this.original.readLong();
        length = headerLength;

        while (length > 0)
        {

            if (length < buffer.length)
                read = this.original.read(buffer, 0, (int) length);
            else
                read = this.original.read(buffer);

            baos.write(buffer, 0, read);
            length -= read;

        }

        String json = new String(baos.toByteArray(), StandardCharsets.UTF_8);

        publicProps.putAll(GSON.fromJson(json, MAP_STRING_STRING_TYPE));

        this.ivLength = Integer.parseInt(this.publicHeader.get(ENCRYPTION_IV_LENGTH_LABEL));

        this.original.readLong();// data length 0

        // init cipher
        final Cipher cipher = getCipher();

        // read private properties
        this.original.read(buffer, 0, this.ivLength);// read properties iv
        IvParameterSpec iv = new IvParameterSpec(Arrays.copyOf(buffer, this.ivLength));
        cipher.init(Cipher.DECRYPT_MODE, this.encryptionKey, iv);

        final long propLength = this.original.readLong();
        length = propLength;

        baos.reset();

        while (length > 0)
        {

            if (length < buffer.length)
                read = this.original.read(buffer, 0, (int) length);
            else
                read = this.original.read(buffer);

            final int decrypted = cipher.update(buffer, 0, read, outBuffer);

            baos.write(outBuffer, 0, decrypted);
            length -= read;

        }

        baos.write(cipher.doFinal());

        json = new String(baos.toByteArray());

        props.putAll(GSON.fromJson(json, MAP_STRING_STRING_TYPE));
        this.original.readLong();// data length 0

        while (this.original.getFilePointer() < this.original.length())
        {
            baos.reset();

            final long offset = this.original.getFilePointer();

            this.original.read(buffer, 0, this.ivLength);// read properties iv
            iv = new IvParameterSpec(Arrays.copyOf(buffer, this.ivLength));
            cipher.init(Cipher.DECRYPT_MODE, this.encryptionKey, iv);

            final long metaLength = this.original.readLong();
            final long metaOffset = this.original.getFilePointer();

            length = metaLength;
            while (length > 0)
            {

                if (length < buffer.length)
                    read = this.original.read(buffer, 0, (int) length);
                else
                    read = this.original.read(buffer);

                final int decrypted = cipher.update(buffer, 0, read, outBuffer);
                baos.write(outBuffer, 0, decrypted);
                length -= read;
            }
            baos.write(cipher.doFinal());
            json = new String(baos.toByteArray());

            final Map<String, String> properties = new HashMap<>(GSON.fromJson(json, MAP_STRING_STRING_TYPE));
            final String path = properties.get(Block.PATH_LABEL);
            if (path == null)
                throw new IllegalStateException("Path of block starting at " + offset + " is not set");

            if (blocks.containsKey(path.toUpperCase(Environment.getLocale())))
                throw new IllegalStateException("Block path " + path + " already exist");

            final long dataLength = this.original.readLong();
            final long dataOffset = this.original.getFilePointer();

            final String [] tokens = path.split(Folder.REGEX_DELIMITER);

            this.root.mkdir(tokens, 1, true);

            final org.ortis.jsafebox.SafeFile dstFile;

            if (tokens.length == 2)
                dstFile = this.root;
            else
                dstFile = this.root.get(tokens, 1, tokens.length - 1);

            if (dstFile == null)
                throw new Exception("Could not find destination folder for block path " + path);

            if (!dstFile.isFolder())
                throw new Exception("Destination folder " + dstFile + " is a block");

            final Folder destinationFolder = ((Folder) dstFile);

            final long blockLength = original.getFilePointer() - offset + dataLength;
            final Block block = new Block(path, properties, offset, blockLength, metaOffset, metaLength, dataOffset, dataLength, destinationFolder);

            destinationFolder.add(block);

            blocks.put(block.getComparablePath(), block);
            this.original.seek(block.getOffset() + block.getLength());
        }

    }

    /**
     * Add data into the {@link Safe}. <b>Note that the data will be stored into the temporary safe file</b>. Use {@link Safe#save()} to save all temporary data
     * 
     * @param properties:
     *            metadata
     * @param data:
     *            data to encrypt
     * @return
     * @throws Exception
     */
    public synchronized Block add(final Map<String, String> properties, final InputStream data, TaskProbe probe) throws Exception
    {
        if (probe == null)
            probe = TaskProbe.DULL_PROBE;

        try
        {
            final String path = properties.get(Block.PATH_LABEL);

            if (path == null)
                throw new IllegalArgumentException("Property " + Block.PATH_LABEL + " is missing");

            org.ortis.jsafebox.SafeFile destinationFile = SafeFiles.get(path, this.root, this.root);

            if (destinationFile != null)
                throw new Exception("Block file " + destinationFile + " already exist");

            final String comparablePath = properties.get(Block.PATH_LABEL).toUpperCase(Environment.getLocale());

            final String [] comparableTokens = comparablePath.split(Folder.REGEX_DELIMITER);

            if (comparableTokens.length == 2 && root.getComparableName().equals(comparableTokens[0]))
                destinationFile = this.root;
            else
                destinationFile = this.root.get(comparableTokens, 1, comparableTokens.length - 1);

            if (destinationFile == null)
                throw new Exception("Destination folder " + destinationFile + " does not exists");

            if (!destinationFile.isFolder())
                throw new Exception("Destination " + destinationFile + " is not a folder");

            final Folder destinationFolder = (Folder) destinationFile;

            if (this.roBlocks.containsKey(path) || this.tempBlocks.containsKey(path))
                throw new Exception("Block path " + path + " already exist");

            final String name = properties.get(Block.NAME_LABEL);

            if (name == null)
                throw new IllegalArgumentException("Property " + Block.NAME_LABEL + " is missing");

            final Cipher cipher = getCipher();

            if (probe.isCancelRequested())
            {
                probe.fireCanceled();
                throw new CancellationException();
            }

            cipher.init(Cipher.ENCRYPT_MODE, this.encryptionKey, getSecureRandom());

            final RandomAccessFile temp = getTemp();

            final long offset = temp.getFilePointer();

            temp.write(cipher.getIV());

            // write metadata

            temp.writeLong(0);
            final String metadataserial = GSON.toJson(properties);
            final byte [] metaBuffer = metadataserial.getBytes();
            final long metaOffset = temp.getFilePointer();
            final long metaLength = encrypt(new ByteArrayInputStream(metaBuffer), cipher, temp, this.bufferSize, probe);
            long position = temp.getFilePointer();

            temp.seek(offset + cipher.getIV().length);
            temp.writeLong(metaLength);
            temp.seek(position);

            // write data
            position = temp.getFilePointer();
            temp.writeLong(0);

            final long dataOffset = temp.getFilePointer();

            final long dataLength = encrypt(data, cipher, temp, this.bufferSize, probe);

            temp.seek(position);
            temp.writeLong(dataLength);
            temp.seek(temp.length());

            final Block block = new Block(path, properties, offset, temp.getFilePointer() - offset, metaOffset, metaLength, dataOffset, dataLength, destinationFolder);
            this.tempBlocks.put(block.getComparablePath(), block);

            destinationFolder.add(block);

            return block;

        } catch (final CancellationException e)
        {
            throw e;
        } catch (final Exception e)
        {
            probe.fireException(e);
            throw e;
        } finally
        {
            probe.fireTerminated();
        }
    }

    /**
     * Delete data from the {@link Safe}. <b>Note that the data wont be deleted until a call to {@link Safe#save()} is made</b>
     * 
     * @param path:
     *            path of the data to delete
     */
    public  synchronized  void delete(final String path)
    {

        final String comparablePath = path.toUpperCase(Environment.getLocale());
        Block deleted = this.blocks.get(comparablePath);

        if (deleted != null)
        {
            final Folder folder = deleted.getParent();
            folder.remove(deleted.getName());
            this.deletedBlocks.put(comparablePath, deleted);
        }

        deleted = this.tempBlocks.remove(comparablePath);

        if (deleted != null)
        {
            final Folder folder = deleted.getParent();
            folder.remove(deleted.getName());
            this.deletedBlocks.put(comparablePath, deleted);
        }
    }

    /**
     * Extract data from the {@link Safe}
     * 
     * @param block:
     *            block to extract
     * @param outputStream:
     *            destination of extracted block
     * @throws Exception
     */
    public void extract(final Block block, final OutputStream outputStream) throws Exception
    {
        extract(block.getPath(), outputStream);
    }

    /**
     * Extract data from the {@link Safe}
     * 
     * @param path:
     *            path of the block to extract
     * @param outputStream:
     *            destination of extracted block
     * @throws Exception
     */
    public  synchronized  void extract(String path, final OutputStream outputStream) throws Exception
    {

        path = path.toUpperCase(Environment.getLocale());

        Block block = this.roBlocks.get(path);

        final RandomAccessFile raf;
        if (block == null)
        {
            block = this.tempBlocks.get(path);
            raf = this.temp;
        } else
            raf = this.original;

        if (block == null)
            throw new Exception("Block " + path + " not found");

        raf.seek(block.getOffset());

        final byte [] ivBytes = new byte[this.ivLength];
        raf.read(ivBytes);

        final Cipher cipher = getCipher();

        final IvParameterSpec iv = new IvParameterSpec(ivBytes);

        cipher.init(Cipher.DECRYPT_MODE, this.encryptionKey, iv);
        raf.seek(block.getDataOffset());
        decrypt(raf, block.getDataLength(), cipher, outputStream, this.bufferSize);

    }

    /**
     * Read the metadata of a {@link Block}
     * 
     * @param block:
     *            block to read
     * @return
     * @throws Exception
     */
    public  synchronized  Map<String, String> readMetadata(final Block block) throws Exception
    {

        this.original.seek(block.getOffset());
        final byte [] ivBytes = new byte[this.ivLength];
        this.original.read(ivBytes);

        final Cipher cipher = getCipher();

        final IvParameterSpec iv = new IvParameterSpec(ivBytes);
        cipher.init(Cipher.DECRYPT_MODE, this.encryptionKey, iv);

        this.original.seek(block.getMetaOffset());

        final ByteArrayOutputStream baos = new ByteArrayOutputStream();
        decrypt(this.original, block.getMetaLength(), cipher, baos, this.bufferSize);

        final String metadata = new String(baos.toByteArray());

        final Map<String, String> jsonMap = GSON.fromJson(metadata, MAP_STRING_STRING_TYPE);
        return new TreeMap<>(jsonMap);
    }

    /**
     * Discard pending modification
     */
    public  synchronized  void discardChanges() throws Exception
    {

        for (final Map.Entry<String, Block> temp : this.tempBlocks.entrySet())
        {

            Folder folder = temp.getValue().getParent();
            folder.remove(temp.getValue().getName());
        }

        this.tempBlocks.clear();

        for (final Map.Entry<String, Block> deleted : this.deletedBlocks.entrySet())
        {
            Folder folder = deleted.getValue().getParent();
            folder.add(deleted.getValue());
        }
        this.deletedBlocks.clear();

    }

    /**
     * Save the modification into the safe file. The current file is renamed and a new file is written. This is to reduce the risk of data loss. This method calls the {@link Safe#close()} before returning
     * 
     * @return
     * @throws Exception
     */
    public Safe save() throws Exception
    {
        return save(null);
    }

    public  synchronized  Safe save(TaskProbe probe) throws Exception
    {

        if (probe == null)
            probe = TaskProbe.DULL_PROBE;
        try
        {
            double progress = 0;
            probe.fireProgress(progress);

            probe.fireMessage("Creating temporary file");
            final File newFile = Files.createTempFile(originalFile.getParentFile().toPath(), null, null).toFile();

            try (RandomAccessFile destination = new RandomAccessFile(newFile, "rw"))
            {

                Cipher cipher = getCipher();

                if (probe.isCancelRequested())
                {
                    probe.fireCanceled();
                    throw new CancellationException();
                }

                destination.write(HASHER.getEmptyHash());// skip hash

                // public properties
                probe.fireMessage("Writing public header");

                String json = GSON.toJson(this.publicHeader);

                long previousPosition = destination.getFilePointer();

                destination.writeLong(0);
                long total = write(new ByteArrayInputStream(json.getBytes()), destination, this.bufferSize, probe);
                destination.writeLong(0);// no data in header
                long position = destination.getFilePointer();
                destination.seek(previousPosition);
                destination.writeLong(total);
                destination.seek(position);

                if (probe.isCancelRequested())
                {
                    probe.fireCanceled();
                    throw new CancellationException();
                }

                // private properties
                probe.fireMessage("Writing private properties");

                cipher.init(Cipher.ENCRYPT_MODE, this.encryptionKey, getSecureRandom());
                destination.write(cipher.getIV());

                json = GSON.toJson(this.privateProperties);

                previousPosition = destination.getFilePointer();
                destination.writeLong(0);
                total = encrypt(new ByteArrayInputStream(json.getBytes()), cipher, destination, this.bufferSize, probe);
                destination.writeLong(0);// no data in header
                position = destination.getFilePointer();
                destination.seek(previousPosition);
                destination.writeLong(total);
                destination.seek(position);

                if (probe.isCancelRequested())
                {
                    probe.fireCanceled();
                    throw new CancellationException();
                }

                final double steps = this.roBlocks.size() + this.tempBlocks.size() + 1;
                int completed = 0;

                for (final Block block : this.roBlocks.values())
                {
                    // add non deleted only
                    if (this.deletedBlocks.containsKey(block.getComparablePath()))
                    {
                        probe.fireMessage("Skipping deleted block " + block.getPath());
                        continue;
                    }

                    probe.fireMessage("Writing block " + block.getPath());
                    this.original.seek(block.getOffset());
                    write(this.original, block.getLength(), destination, this.bufferSize, probe);
                    completed++;
                    progress = completed / steps;
                    probe.fireProgress(progress);
                }

                final RandomAccessFile temp = getTemp();
                for (final Block block : this.tempBlocks.values())
                {

                    if (this.deletedBlocks.containsKey(block.getComparablePath()))
                    {
                        probe.fireMessage("Skipping deleted block " + block.getPath());
                        continue;
                    }

                    probe.fireMessage("Writing block " + block.getPath());
                    temp.seek(block.getOffset());

                    write(temp, block.getLength(), destination, this.bufferSize, probe);
                    completed++;
                    progress = completed / steps;
                    probe.fireProgress(progress);

                }

                probe.fireMessage("Computing hash");
                final byte [] hash = computeHash(destination, cipher, this.ivLength, this.encryptionKey, this.bufferSize, probe);
                destination.seek(0);
                destination.write(hash);

                probe.fireMessage("Closing IO streams");
                destination.close();

                close();

                probe.fireMessage("Deleting old file");

                if (!this.originalFile.delete())
                    throw new IOException("Unable to delete " + this.originalFile.getAbsolutePath());

                probe.fireMessage("Renaming file");

                if (!newFile.renameTo(this.originalFile))
                    throw new IOException("Unable to rename " + newFile.getAbsolutePath());

                if (probe.isCancelRequested())
                {
                    probe.fireCanceled();
                    throw new CancellationException();
                }

                probe.fireMessage("Opening new safe");
                probe.fireProgress(1);

                return new Safe(this.originalFile, encryptionKey, this.bufferSize);
            }
        } catch (final CancellationException e)
        {
            throw e;
        } catch (final Exception e)
        {
            probe.fireException(e);
            throw e;
        } finally
        {
            probe.fireTerminated();
        }

    }

    /**
     * Compute the hash of the {@link Safe}
     * 
     * @return
     * @throws Exception
     */
    public  synchronized  byte [] computeHash(final TaskProbe probe) throws Exception
    {
        final byte [] hash = computeHash(this.original, getCipher(), this.ivLength, this.encryptionKey, this.bufferSize, probe);
        return hash;
    }

    /**
     * Return a copy of the hash that was in the {@link Safe}'s file
     * 
     * @return
     */
    public byte [] getHash()
    {
        final byte [] destination = new byte[this.hash.length];
        System.arraycopy(this.hash, 0, destination, 0, this.hash.length);
        return destination;
    }

    private Cipher getCipher() throws Exception
    {
        final String encryption = this.publicHeader.get(ENCRYPTION_LABEL);

        if (encryption == null)
            throw new Exception("Public property '" + ENCRYPTION_LABEL + "' must be set");

        return javax.crypto.Cipher.getInstance(encryption);
    }

    @Override
    public  synchronized  void close() throws IOException
    {
        this.original.close();

        final RandomAccessFile temp = getTemp();
        if (temp != null)
        {
            temp.close();
            tempFile.delete();
        }

    }

    /**
     * Get the properties of the {@link Safe}
     * 
     * @return
     */
    public Map<String, String> getPrivateProperties()
    {
        return privateProperties;
    }

    /**
     * Get the header of the {@link Safe}
     * 
     * @return
     */
    public Map<String, String> getPublicHeader()
    {
        return publicHeader;
    }

    /**
     * Get all {@link Block} contained in the {@link Safe}
     * 
     * @return
     */
    public Map<String, Block> getBlocks()
    {
        return this.roBlocks;
    }

    /**
     * Get a {@link Block} from the {@link Safe}
     * 
     * @param path:
     *            path of the {@link Block} to retrieve
     * @return
     */
    public Block getBlock(final String path)
    {

        final String comparablePath = path.toUpperCase(Environment.getLocale());

        return this.roBlocks.get(comparablePath);

    }

    /**
     * Get a {@link Block} from the temporary {@link Safe}
     * 
     * @param path:
     *            path of the {@link Block} to retrieve
     * @return
     */
    public Block getTempBlock(final String path)
    {
        final String comparablePath = path.toUpperCase(Environment.getLocale());

        return this.tempBlocks.get(comparablePath);

    }

    /**
     * Get all {@link Block} contained in the temporary {@link Safe}
     * 
     * @return
     */
    public Map<String, Block> getTempBlocks()
    {
        return tempBlocks;
    }

    /**
     * Get deleted {@link Block}
     * 
     * @return
     */
    public Map<String, Block> getDeletedBlocks()
    {
        return deletedBlocks;
    }

    /**
     * Get root {@link Folder}
     * 
     * @return
     */
    public Folder getRootFolder()
    {
        return root;
    }

    public File getFile()
    {
        return this.originalFile;
    }

    /**
     * Get the temporary safe file
     * 
     * @return
     */
    public File getTempFile()
    {
        return tempFile;
    }

    /**
     * Get the temporary safe file
     * 
     * @return
     */
    public RandomAccessFile getTemp() throws IOException
    {
        return this.temp;
    }

    private static long encrypt(final InputStream data, final Cipher cipher, final RandomAccessFile destination, final int bufferSize, final TaskProbe probe) throws Exception
    {

        final byte [] buffer = new byte[bufferSize];
        final byte [] bufferOut = new byte[bufferSize];

        long total = 0;
        int read;
        while ((read = data.read(buffer)) > -1)
        {

            read = cipher.update(buffer, 0, read, bufferOut);
            if (read == 0)
                // data length is less than cipher block size
                System.arraycopy(buffer, 0, bufferOut, 0, buffer.length);

            total += read;
            destination.write(bufferOut, 0, read);

            if (probe.isCancelRequested())
            {
                probe.fireCanceled();
                throw new CancellationException();
            }

        }

        read = cipher.doFinal(bufferOut, 0);
        destination.write(bufferOut, 0, read);
        total += read;

        return total;

    }

    private static void decrypt(final RandomAccessFile source, final long length, final Cipher cipher, final OutputStream destination, final int bufferSize) throws Exception
    {

        final byte [] buffer = new byte[bufferSize];
        final byte [] bufferOut = new byte[bufferSize];

        long remaining = length;
        int read;
        while (remaining > 0)
        {
            if (remaining < buffer.length)
                read = source.read(buffer, 0, (int) remaining);
            else
                read = source.read(buffer, 0, buffer.length);

            remaining -= read;

            read = cipher.update(buffer, 0, read, bufferOut);
            destination.write(bufferOut, 0, read);

        }

        read = cipher.doFinal(bufferOut, 0);
        destination.write(bufferOut, 0, read);

    }

    private static long write(final InputStream data, final RandomAccessFile destination, final int bufferSize, final TaskProbe probe) throws Exception
    {

        final byte [] buffer = new byte[bufferSize];

        long total = 0;
        int read;
        while ((read = data.read(buffer)) > -1)
        {
            destination.write(buffer, 0, read);
            total += read;

            if (probe.isCancelRequested())
            {
                probe.fireCanceled();
                throw new CancellationException();
            }
        }

        return total;

    }

    private static void write(final RandomAccessFile source, final long length, final RandomAccessFile destination, final int bufferSize, final TaskProbe probe) throws Exception
    {

        final byte [] buffer = new byte[bufferSize];

        long remaining = length;
        int read;
        while (remaining > 0)
        {
            if (remaining < buffer.length)
                read = source.read(buffer, 0, (int) remaining);
            else
                read = source.read(buffer, 0, buffer.length);

            destination.write(buffer, 0, read);

            remaining -= read;

            if (probe.isCancelRequested())
            {
                probe.fireCanceled();
                throw new CancellationException();
            }
        }

    }

    private static SecureRandom getSecureRandom()
    {
        return new SecureRandom();
    }

    /**
     * Read the header of the {@link Safe}
     * 
     * @param file:
     *            safe file to read
     * @param bufferSize:
     *            size of the <code>byte</code> buffer to be used in IO operation
     * @return
     * @throws IOException
     */
    public static Map<String, String> readHeader(final File file, final int bufferSize) throws IOException
    {
        RandomAccessFile raf = null;

        try
        {
            raf = new RandomAccessFile(file, "rw");
            final byte [] buffer = new byte[bufferSize];
            final ByteArrayOutputStream baos = new ByteArrayOutputStream(buffer.length);

            raf.read(buffer, 0, HASHER.getHashLength());// skip hash

            long length = raf.readLong();
            int read;
            while (length > 0)
            {

                if (length < buffer.length)
                    read = raf.read(buffer, 0, (int) length);
                else
                    read = raf.read(buffer);

                baos.write(buffer, 0, read);
                length -= read;

            }
            final String header = new String(baos.toByteArray(), StandardCharsets.UTF_8);
            return GSON.fromJson(header, MAP_STRING_STRING_TYPE);

        } finally
        {
            if (raf != null)
                raf.close();
        }

    }

    /**
     * Compute the hash value of {@link Safe} file
     * 
     * @param safeFile
     * @param cipher
     * @param ivLength
     * @param encryptionKey
     * @param bufferSize
     * @return
     * @throws Exception
     */
    public static byte [] computeHash(final RandomAccessFile safeFile, final Cipher cipher, final int ivLength, final Key encryptionKey, final int bufferSize, TaskProbe probe) throws Exception
    {

        if (probe == null)
            probe = TaskProbe.DULL_PROBE;

        try
        {
            final long previousPosition = safeFile.getFilePointer();

            final byte [] buffer = new byte[bufferSize];
            final byte [] bufferOut = new byte[bufferSize];


            safeFile.seek(HASHER.getHashLength());
            final ByteBuffer byteBuffer = ByteBuffer.allocate((int) (safeFile.length() - safeFile.getFilePointer()));

            long length = safeFile.readLong();
            byteBuffer.putLong(length);

            int read;

            // header
            while (length > 0)// read header
            {

                if (length < buffer.length)
                    read = safeFile.read(buffer, 0, (int) length);
                else
                    read = safeFile.read(buffer);

                byteBuffer.put(buffer, 0, read);
                length -= read;
            }

            byteBuffer.putLong(safeFile.readLong());// header's data length 0

            if (probe.isCancelRequested())
            {
                probe.fireCanceled();
                throw new CancellationException();
            }

            // properties
            safeFile.read(buffer, 0, ivLength);// read properties iv
            byteBuffer.put(buffer, 0, ivLength);

            IvParameterSpec iv = new IvParameterSpec(Arrays.copyOf(buffer, ivLength));
            cipher.init(Cipher.DECRYPT_MODE, encryptionKey, iv);

            length = safeFile.readLong();
            byteBuffer.putLong(length);

            while (length > 0)// read properties
            {
                if (length < buffer.length)
                    read = safeFile.read(buffer, 0, (int) length);
                else
                    read = safeFile.read(buffer, 0, buffer.length);

                length -= read;

                read = cipher.update(buffer, 0, read, bufferOut);
                byteBuffer.put(bufferOut, 0, read);

            }

            read = cipher.doFinal(bufferOut, 0);
            byteBuffer.put(bufferOut, 0, read);

            byteBuffer.putLong(safeFile.readLong());// properties data length 0

            if (probe.isCancelRequested())
            {
                probe.fireCanceled();
                throw new CancellationException();
            }

            // blocks
            while (safeFile.getFilePointer() < safeFile.length())
            {
                safeFile.read(buffer, 0, ivLength);// read properties iv
                byteBuffer.put(buffer, 0, ivLength);
                iv = new IvParameterSpec(Arrays.copyOf(buffer, ivLength));
                cipher.init(Cipher.DECRYPT_MODE, encryptionKey, iv);

                // read metadata
                length = safeFile.readLong();
                byteBuffer.putLong(length);

                while (length > 0)
                {
                    if (length < buffer.length)
                        read = safeFile.read(buffer, 0, (int) length);
                    else
                        read = safeFile.read(buffer, 0, buffer.length);

                    length -= read;

                    read = cipher.update(buffer, 0, read, bufferOut);
                    byteBuffer.put(bufferOut, 0, read);

                }

                read = cipher.doFinal(bufferOut, 0);
                byteBuffer.put(bufferOut, 0, read);

                if (probe.isCancelRequested())
                {
                    probe.fireCanceled();
                    throw new CancellationException();
                }

                // read data
                length = safeFile.readLong();
                byteBuffer.putLong(length);

                while (length > 0)
                {
                    if (length < buffer.length)
                        read = safeFile.read(buffer, 0, (int) length);
                    else
                        read = safeFile.read(buffer, 0, buffer.length);

                    length -= read;

                    read = cipher.update(buffer, 0, read, bufferOut);
                    byteBuffer.put(bufferOut, 0, read);

                    if (probe.isCancelRequested())
                    {
                        probe.fireCanceled();
                        throw new CancellationException();
                    }

                }

                read = cipher.doFinal(bufferOut, 0);
                byteBuffer.put(bufferOut, 0, read);

            }

            safeFile.seek(previousPosition);

            return HASHER.hash(byteBuffer.array());

        } catch (final CancellationException e)
        {
            throw e;
        } catch (final Exception e)
        {
            probe.fireException(e);
            throw e;
        } finally
        {
            probe.fireTerminated();
        }
    }

    /**
     * Create a new {@link Safe}
     * 
     * @param file
     * @param key
     * @param publicHeader
     * @param privateProperties
     * @param bufferSize
     * @return
     * @throws Exception
     */
    public static Safe create(final File file, final byte [] key, final Map<String, String> publicHeader, final Map<String, String> privateProperties, final int bufferSize) throws Exception
    {

        final String encryption = publicHeader.get(ENCRYPTION_LABEL);

        if (encryption == null)
            throw new Exception("Public property '" + ENCRYPTION_LABEL + "' must be set");

        if (!publicHeader.containsKey(ENCRYPTION_IV_LENGTH_LABEL))
            throw new Exception("Public property '" + ENCRYPTION_IV_LENGTH_LABEL + "' must be set");

        if (!publicHeader.containsKey(PBKDF2_SALT_LABEL))
            throw new Exception("Public property '" + PBKDF2_SALT_LABEL + "' must be set");

        if (!publicHeader.containsKey(PBKDF2_ITERATION_LABEL))
            throw new Exception("Public property '" + PBKDF2_ITERATION_LABEL + "' must be set");

        Cipher cipher = javax.crypto.Cipher.getInstance(encryption);

        final String keyAlgo = publicHeader.get(KEY_ALGO_LABEL);
        if (keyAlgo == null)
            throw new Exception("Public property '" + KEY_ALGO_LABEL + "' must be set");

        final SecretKeySpec keySpec = new SecretKeySpec(key, keyAlgo);

        if (file.exists())
            throw new IOException("File " + file + " already exist");

        if (!file.createNewFile())
            throw new IOException("Could not create file " + file);

        final RandomAccessFile raf = new RandomAccessFile(file, "rw");

        long total, position, previousPosition;

        cipher.init(Cipher.ENCRYPT_MODE, keySpec, getSecureRandom());

        raf.write(HASHER.getEmptyHash());// global hash

        // header
        position = raf.getFilePointer();
        // no IV in header
        raf.writeLong(0);
        final String header = GSON.toJson(publicHeader);
        total = write(new ByteArrayInputStream(header.getBytes(StandardCharsets.UTF_8)), raf, bufferSize, TaskProbe.DULL_PROBE);
        raf.writeLong(0);// no data in header block

        previousPosition = raf.getFilePointer();
        raf.seek(position);
        raf.writeLong(total);

        raf.seek(previousPosition);

        // properties
        position = raf.getFilePointer();
        raf.write(cipher.getIV());
        final String privatePropsJson = GSON.toJson(privateProperties == null ? new HashMap<>() : privateProperties);
        previousPosition = raf.getFilePointer();
        raf.writeLong(0L);
        total = encrypt(new ByteArrayInputStream(privatePropsJson.getBytes(StandardCharsets.UTF_8)), cipher, raf, bufferSize, TaskProbe.DULL_PROBE);
        raf.writeLong(0);// no data in properties block

        raf.seek(previousPosition);
        raf.writeLong(total);

        // write global hash
        final byte [] hash = computeHash(raf, cipher, cipher.getIV().length, keySpec, bufferSize, null);
        raf.seek(0);
        raf.write(hash);

        raf.close();

        return new Safe(file, keySpec, bufferSize);

    }

}

org/ortis/jsafebox/Utils.java

/*******************************************************************************
 * Copyright 2018 Ortis (cao.ortis.org@gmail.com)
 * 
 * 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.ortis.jsafebox;

import java.awt.GraphicsDevice;
import java.awt.GraphicsEnvironment;
import java.awt.HeadlessException;
import java.io.File;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.CharBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystems;
import java.nio.file.FileVisitResult;
import java.nio.file.FileVisitor;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.PathMatcher;
import java.nio.file.Paths;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.logging.Logger;
import java.util.regex.Pattern;

import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;
import javax.crypto.spec.SecretKeySpec;

/**
 * Utility class
 * 
 * @author Ortis <br>
 *         2018 Apr 26 8:06:47 PM <br>
 */
public class Utils
{

    public final static String SEPARATOR_REGEX = "[/|" + Pattern.quote(java.io.File.separator) + "]";

    private final static String SYSTEM_PATH_DELIMITER_REGEX = Pattern.quote(File.separator) + "|" + Pattern.quote("/") + "|" + Pattern.quote("\\");

    public static byte [] passwordToBytes(final char [] chars)
    {
        final CharBuffer charBuffer = CharBuffer.wrap(chars);
        final ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(charBuffer);
        final byte [] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit());
        Arrays.fill(charBuffer.array(), '\u0000'); // clear sensitive data
        Arrays.fill(byteBuffer.array(), (byte) 0); // clear sensitive data
        return bytes;
    }

    /**
     * Open a {@link Safe}
     * 
     * @param safeFilePath:
     *            system path to the safe file
     * @param password:
     *            the encryption password
     * @param bufferSize:size
     *            of the <code>byte</code> buffer to be used in IO operation
     * @param log
     * @return
     * @throws Exception
     */
    public static Safe open(final String safeFilePath, final char [] password, final int bufferSize, final Logger log) throws Exception
    {
        final File file = new File(safeFilePath);

        if (!file.exists())
            throw new IOException("Safe file " + file + " doest not exist");

        final Map<String, String> header = Safe.readHeader(file, bufferSize);

        final String encyption = header.get(Safe.ENCRYPTION_LABEL);
        if (encyption == null)
            throw new Exception("Could not read property '" + Safe.ENCRYPTION_LABEL + "' from header");

        if (log != null)
            log.fine("Encryption type " + encyption);

        if (!header.containsKey(Safe.KEY_ALGO_LABEL))
            throw new Exception("Could not read property '" + Safe.KEY_ALGO_LABEL + "' from header");

        if (log != null)
            log.fine("Key algorithm " + header.get(Safe.KEY_ALGO_LABEL));


        final byte [] salt = (byte [])Safe.GSON.fromJson(header.get(Safe.PBKDF2_SALT_LABEL), Safe.BYTE_ARRAY_TYPE);

        PBEKeySpec spec = new PBEKeySpec(password, salt, Safe.PBKDF2_ITERATION, 128);
        SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");
        final byte [] key = skf.generateSecret(spec).getEncoded();

        final SecretKeySpec keySpec = new SecretKeySpec(key, header.get(Safe.KEY_ALGO_LABEL));

        return new Safe(file, keySpec, bufferSize);
    }

    public static List<java.io.File> parseSystemPath(String query, final List<java.io.File> destination) throws IOException
    {
        final String [] tokens = query.split(SYSTEM_PATH_DELIMITER_REGEX);

        Path baseDirectory = null;

        if (tokens[0].equals(".") || tokens[0].equals(".."))
        {
            baseDirectory = new File(tokens[0]).toPath();

            final StringBuilder sb = new StringBuilder();
            for (int i = 1; i < tokens.length; i++)
                if (sb.length() == 0)
                    sb.append(tokens[i]);
                else
                    sb.append(File.separator + tokens[i]);

            query = "**" + File.separator + sb.toString();

        } else
        {

            final String comparableToken = tokens[0].toUpperCase();
            for (final File root : File.listRoots())
                if (root.getAbsolutePath().toUpperCase().equals(comparableToken))
                {
                    // perfect match
                    baseDirectory = root.toPath();
                    break;

                }

            if (baseDirectory == null)
                for (final File root : File.listRoots())
                {
                    String rootPath = root.getAbsolutePath().toUpperCase();
                    rootPath = rootPath.substring(0, rootPath.length() - 1);
                    if (rootPath.equals(comparableToken))
                    {
                        baseDirectory = root.toPath();
                        break;
                    }

                }
        }

        if (baseDirectory == null)
            throw new IOException("Could not locate base directory '" + tokens[0] + "'");

        Path path = baseDirectory;
        for (int i = 1; i < tokens.length; i++)
        {

            try
            {
                path = Paths.get(path.toString(), tokens[i]);
            } catch (final Exception e)
            {
                // here, we have reach a special character and the start point for the search is
                // in path
            }
        }

        final String escapedQuery = query.replace("\\", "\\\\");// PathMatcher does not escape backslash properly. Need to do the escape manually for Windows OS path handling. This might be a bug of Java implentation.
        // Need to check on Oracle bug report database.

        final PathMatcher pathMatcher = FileSystems.getDefault().getPathMatcher("glob:" + escapedQuery);
        Files.walkFileTree(path, new FileVisitor<Path>()
        {

            @Override
            public FileVisitResult postVisitDirectory(final Path dir, final IOException exc) throws IOException
            {

                return FileVisitResult.CONTINUE;
            }

            @Override
            public FileVisitResult preVisitDirectory(final Path dir, final BasicFileAttributes attrs) throws IOException
            {
                if (pathMatcher.matches(dir))
                {
                    destination.add(dir.toFile());
                    return FileVisitResult.SKIP_SUBTREE;
                }

                return FileVisitResult.CONTINUE;
            }

            @Override
            public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException
            {

                if (pathMatcher.matches(file))
                    destination.add(file.toFile());

                return FileVisitResult.CONTINUE;
            }

            @Override
            public FileVisitResult visitFileFailed(final Path file, final IOException exc) throws IOException
            {
                return FileVisitResult.CONTINUE;
            }
        });

        return destination;

    }

    /**
     * Return the MIME type of a file
     * 
     * @param file
     * @return
     */
    public static String getMIMEType(final java.io.File file)
    {

        final String name = file.getName().toUpperCase();

        if (name.endsWith(".TXT"))
            return "text/plain";
        else if (name.endsWith(".CSV"))
            return "text/csv";
        else if (name.endsWith(".HTM") || name.endsWith(".HTML"))
            return "text/html";
        else if (name.endsWith(".JPG") || name.endsWith(".JPEG"))
            return "image/jpg";
        else if (name.endsWith(".PNG"))
            return "image/png";
        else if (name.endsWith(".BM") || name.endsWith(".BMP"))
            return "image/bmp";
        else if (name.endsWith(".PDF"))
            return "application/pdf";
        else if (name.endsWith(".AVI"))
            return "video/x-msvideo";
        else if (name.endsWith(".MPEG"))
            return "video/mpeg";
        else if (name.endsWith(".MP4"))
            return "video/mp4";
        else if (name.endsWith(".MKV"))
            return "video/x-matroska";
        else if (name.endsWith(".MP3"))
            return "audio/mpeg";
        else
            return "application/octet-stream";
    }

    /**
     * Format the exception message
     * 
     * @param t
     * @return
     */
    public static String formatException(final Throwable t)
    {
        if (t == null)
            return null;

        final Throwable cause = t.getCause();
        final String msg = cause == null ? null : formatException(cause);
        return formatException(t.getClass(), msg, t.toString(), t.getStackTrace());

    }

    private static String formatException(final Class<?> exceptionClass, final String cause, final String msg, final StackTraceElement [] exceptionStack)
    {
        final StringBuilder builder = new StringBuilder();

        if (msg != null)
            builder.append(msg);

        if (exceptionStack != null)
        {
            builder.append(System.lineSeparator());
            for (int i = 0; i < exceptionStack.length; i++)
            {
                final String stackElement = exceptionStack[i].toString();

                builder.append(stackElement + System.lineSeparator());
            }
        }

        if (cause != null)
            builder.append("Caused by " + cause);

        return builder.toString();
    }

    /**
     * Remove forbidden <code>char</code> from the path and replace them with <code>substitute</code>
     * 
     * @param path:
     *            the path to sanitize
     * @param delimiter:
     *            delimiter of the path
     * @param substitute:
     *            replacement char
     * @return
     */
    public static String sanitize(final String path, final Character delimiter, final Character substitute)
    {
        final String [] tokens = path.split(Pattern.quote(Character.toString(delimiter)));

        final StringBuilder sb = new StringBuilder();
        for (int i = 0; i < tokens.length; i++)
        {
            if (i < tokens.length - 1)
                sb.append(sanitizeToken(tokens[i], substitute) + delimiter);
            else
                sb.append(sanitizeToken(tokens[i], substitute));

        }

        return sb.toString();
    }

    public static String sanitizeToken(final String token, final Character substitute)
    {

        final StringBuilder sb = new StringBuilder(token);

        final Character replacement = substitute;

        c: for (int i = 0; i < sb.length(); i++)
        {

            if (sb.charAt(i) == java.io.File.separatorChar || sb.charAt(i) == Folder.DELIMITER)
            {
                if (replacement == null)
                    sb.deleteCharAt(i--);
                else
                    sb.setCharAt(i, replacement);
                continue c;
            }

            for (final char c : Environment.getForbidenChars())
                if (sb.charAt(i) == c)
                {
                    if (replacement == null)
                        sb.deleteCharAt(i--);
                    else
                        sb.setCharAt(i, replacement);
                    continue c;
                }
        }
        return sb.toString();

    }

    public static boolean isHeadless()
    {
        if (GraphicsEnvironment.isHeadless())
            return true;

        try
        {
            GraphicsDevice [] screenDevices = GraphicsEnvironment.getLocalGraphicsEnvironment().getScreenDevices();
            return screenDevices == null || screenDevices.length == 0;
        } catch (HeadlessException e)
        {
            return true;
        }
    }

}
  • 1
    Reminder: By posting the code to this site, you're dual-licensing it as CC-BY-SA 3.0 :) – Vogel612 Jun 12 at 8:16
  • Could you please break the class into smaller classes and try to write small methods? You can reuse the methods and reduce the duplicate code in your classes. – Ankit Soni Jun 16 at 2:07
  • @AnkitSoni Those are the core logic classes and they are by far the longest in the project. Breaking them down would be difficult. Normally, there should be be any duplicate code. Please highlight if you spot some. – ortis Jun 16 at 4:31
up vote 1 down vote accepted
+50

Safe.java

  1. final long headerLength = this.original.readLong(); is not used anywhere and its value is immediately assigned to another variable.
  2. Same with final long propLength = this.original.readLong();
  3. Line no 190-200, 233-234 and 905-917 are duplicated.
  4. Line no 1090-1102 and 1127-1139 are duplicated.
  5. destinationFile will be always null at line 316.
  6. Block

    if (probe.isCancelRequested()) 
    {
        probe.fireCanceled();
        throw new CancellationException();
    }
    

    is duplicated 12 times.

  7. Line no 404-409 and 413-418 are duplicated.

  8. Line 995 raf = new RandomAccessFile(file, "rw"); can be closed with try-with-resources, instead of a finally block.
  9. MessageFormat can be used to create all the Exception messages, as the description is almost duplicated. Eg line 1212-1222
  10. The code is not very readable as big methods are not divided into smaller well-named methods, that tell whats happening in the block. Same goes for Utils class too.
  • I did not realized there was so many duplicates. headerLength and propLength exist for debuging reasons and will be removed after the first production version (not beta). Regarding point (5), I assure you it is not always null :) – ortis Jun 18 at 6:40
  • But you are throwing an exception only when it's null, if (destinationFile == null) – Ankit Soni Jun 18 at 6:47
  • Yes because it can be null, but not always – ortis Jun 18 at 6:50
  • Yes, agreed. But printing the value of destinationFile won't make any sense as it will be null. "Destination folder null does not exist" – Ankit Soni Jun 18 at 6:52
  • My bad, I though you were questioning the need of if (destinationFile == null). Yep you right, exception message should be changed. – ortis Jun 18 at 6:54

Just a few notes:

The methods are far to long to be reviewed. This is especially problematic with security-related code. You seem to believe that the methods can't be reasonably split, but this is actually never true for methods over say 30 lines.

Concerning Safe::new, the following lines do not depend on the arguments and may be moved to initializer expressions:

    final HashMap<String, String> publicProps = new LinkedHashMap<>();
    this.publicHeader = Collections.unmodifiableMap(publicProps);
    final HashMap<String, String> props = new LinkedHashMap<>();
    this.privateProperties = Collections.unmodifiableMap(props);
    this.blocks = new LinkedHashMap<>();
    this.roBlocks = Collections.unmodifiableMap(blocks);
    this.tempBlocks = new LinkedHashMap<>();
    this.deletedBlocks = new LinkedHashMap<>();
    this.root = new Folder(null, Folder.ROOT_NAME);

These variables should go or be much more narrowly scoped:

    long length;
    int read;
    final ByteArrayOutputStream baos = new ByteArrayOutputStream(buffer.length);

Actually, local variables without an initializer expression should be extremely rare (I'd bet I have less than one per class on the average). While uninitialized local variables pose no risk in Java, having unassigned variables hampers readability (this is no Pascal, really!).

You could extract a couple of methods like

  • readHeader
  • readProp (whatever it means!)
  • originaltoJson (whatever it means!)

These methods would have quite a few arguments (baos, buffer, outBuffer, ...), which would make them ugly, but there's a simple solution: Create a helper class having these buffers as members (or use a parameter object; or maybe just drop them as not every optimization is worth it).

You're IMHO overusing blank lines, e.g., getBlock is a two-line method having additional three blank lines. This isn't helpful.

You're violating quite a few Java conventions, but I'm sure, you were already told, so I'm not elaborating on this.

  • Can you explained what you mean by 'initializer expressions' ? Instantiating a new ByteArrayOutputStream for each iteration would not be efficient. Blank line should have been removed by the code formatter indeed. Can you develop on the missed Java conventions ? I usually try to follow them. – ortis Jun 18 at 6:47
  • I mean "an assignment in the declaration", I guess the correct name is VariableInitializer. +++ Use a helper class containing all the things you want to recycle for efficiency. +++ I personally strive for methods below 20 lines split into blocks of 2-5 lines by a simple blank line. +++ Conventions: 1. Egyptian brace style, 2. use braces even for single statements. – maaartinus Jun 18 at 11:47
  • Is it wise to use a class to hold one single object ByteArrayOutputStream ? I cannot stand Egyptian brackets, they make the code less readable. Looking at Java source code am not sure they are part of the convention. Do you have any source ? – ortis Jun 18 at 14:47
  • No, if all you want ist the baos, then you surely need no holder. I was assuming that you want to pass the two buffers as well. +++ Sure, oracle.com/technetwork/java/codeconventions-150003.pdf and google.github.io/styleguide/javaguide.html, which is sort of the new standard. Egyptian braces are prevalent in Java and maybe you could get used to them. I personally like them, but I violate conventions in some aspects myself. As long as you're your own boss, it's your choice, I only wanted to inform you. – maaartinus Jun 18 at 19:49
  • I appreciate your inputs. I will definitely update the code to breakdown lengthy methods into shorter one. – ortis Jun 19 at 0:43

Your Answer

 
discard

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.