Always pass charset in String constructor; don't treat strings as binary safe

14 files changed, 57 insertions, 29 deletions

diff --git a/src/main/java/org/openslx/libvirt/domain/device/Video.java b/src/main/java/org/openslx/libvirt/domain/device/Video.java
index f3ee13f..a674715 100644
--- a/src/main/java/org/openslx/libvirt/domain/device/Video.java
+++ b/src/main/java/org/openslx/libvirt/domain/device/Video.java
@@ -73,8 +73,8 @@ public class Video extends Device
 				// only set acceleration on supported Virtio GPUs
 				this.setXmlElementAttributeValueYesNo( "model/acceleration", "accel2d", acceleration );
 			} else {
-				String errorMsg = new String(
-						"Video card model '" + model.toString() + "' does not support enabled 2D hardware acceleration." );
+				String errorMsg =
+						"Video card model '" + model.toString() + "' does not support enabled 2D hardware acceleration.";
 				throw new IllegalArgumentException( errorMsg );
 			}
 		}
@@ -103,8 +103,8 @@ public class Video extends Device
 			// only set acceleration on supported Virtio GPUs
 			this.setXmlElementAttributeValueYesNo( "model/acceleration", "accel3d", acceleration );
 		} else {
-			String errorMsg = new String(
-					"Video card model '" + model.toString() + "' does not support enabled 3D hardware acceleration." );
+			String errorMsg =
+					"Video card model '" + model.toString() + "' does not support enabled 3D hardware acceleration.";
 			throw new IllegalArgumentException( errorMsg );
 		}
 	}
diff --git a/src/main/java/org/openslx/libvirt/xml/LibvirtXmlDocument.java b/src/main/java/org/openslx/libvirt/xml/LibvirtXmlDocument.java
index 97180e3..c428988 100644
--- a/src/main/java/org/openslx/libvirt/xml/LibvirtXmlDocument.java
+++ b/src/main/java/org/openslx/libvirt/xml/LibvirtXmlDocument.java
@@ -10,6 +10,7 @@ import java.io.StringWriter;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.OutputKeys;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerConfigurationException;
 import javax.xml.transform.TransformerException;
@@ -74,7 +75,7 @@ public abstract class LibvirtXmlDocument implements LibvirtXmlSerializable, Libv
 			domFactory.setNamespaceAware( true );
 			this.domBuilder = domFactory.newDocumentBuilder();
 		} catch ( ParserConfigurationException e ) {
-			String errorMsg = new String( "Setting up XML context for reading from the Libvirt XML document failed." );
+			String errorMsg = "Setting up XML context for reading from the Libvirt XML document failed.";
 			throw new LibvirtXmlDocumentException( errorMsg );
 		}
 
@@ -91,8 +92,9 @@ public abstract class LibvirtXmlDocument implements LibvirtXmlSerializable, Libv
 			InputStream xslOutputSchemaStream = LibvirtXmlResources.getLibvirtXsl( "xml-output-transformation.xsl" );
 			StreamSource xslOutputSchema = new StreamSource( xslOutputSchemaStream );
 			this.xmlTransformer = transformerFactory.newTransformer( xslOutputSchema );
+			this.xmlTransformer.setOutputProperty( OutputKeys.ENCODING, "UTF-8" );
 		} catch ( TransformerConfigurationException e ) {
-			String errorMsg = new String( "Setting up XML context for writing to the Libvirt XML document failed." );
+			String errorMsg = "Setting up XML context for writing to the Libvirt XML document failed.";
 			throw new LibvirtXmlDocumentException( errorMsg );
 		}
 
@@ -101,7 +103,7 @@ public abstract class LibvirtXmlDocument implements LibvirtXmlSerializable, Libv
 			try {
 				this.rngValidator = new LibvirtXmlSchemaValidator( rngSchema );
 			} catch ( SAXException e ) {
-				String errorMsg = new String( "Setting up XML context for validating to the Libvirt XML document failed." );
+				String errorMsg = "Setting up XML context for validating to the Libvirt XML document failed.";
 				e.printStackTrace();
 				throw new LibvirtXmlDocumentException( errorMsg );
 			}
diff --git a/src/main/java/org/openslx/libvirt/xml/LibvirtXmlSchemaValidator.java b/src/main/java/org/openslx/libvirt/xml/LibvirtXmlSchemaValidator.java
index bc8f90f..d589d41 100644
--- a/src/main/java/org/openslx/libvirt/xml/LibvirtXmlSchemaValidator.java
+++ b/src/main/java/org/openslx/libvirt/xml/LibvirtXmlSchemaValidator.java
@@ -6,6 +6,7 @@ import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Reader;
+import java.nio.charset.StandardCharsets;
 
 import javax.xml.XMLConstants;
 import javax.xml.transform.Transformer;
@@ -106,7 +107,7 @@ class LibvirtXmlSchemaResourceInput implements LSInput
 				int inputLength = this.inputStream.available();
 				byte[] input = new byte[ inputLength ];
 				this.inputStream.read( input );
-				data = new String( input );
+				data = new String( input, StandardCharsets.UTF_8 );
 			} catch ( IOException e ) {
 				e.printStackTrace();
 			}
diff --git a/src/main/java/org/openslx/util/ThriftUtil.java b/src/main/java/org/openslx/util/ThriftUtil.java
index a9035c1..ef10798 100644
--- a/src/main/java/org/openslx/util/ThriftUtil.java
+++ b/src/main/java/org/openslx/util/ThriftUtil.java
@@ -41,6 +41,7 @@ public class ThriftUtil {
 		BufferedReader reader;
 		StringBuffer content = new StringBuffer("");
 		try {
+			// Why is a generic function to convert a buffer to string CALLING A VMWARE SPECIFIC FUNCTION!?
 			reader = VirtualizationConfigurationVmwareFileFormat.getVmxReader(bytes, bytes.length);
 			String line="";
 			while ((line=reader.readLine()) != null) {
diff --git a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemu.java b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemu.java
index 7082e27..45b4f1f 100644
--- a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemu.java
+++ b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemu.java
@@ -115,7 +115,7 @@ public class VirtualizationConfigurationQemu extends VirtualizationConfiguration
 
 		try {
 			// read and parse Libvirt domain XML configuration document
-			this.vmConfig = new Domain( new String( vmContent ) );
+			this.vmConfig = new Domain( new String( vmContent, StandardCharsets.UTF_8 ) );
 		} catch ( LibvirtXmlDocumentException | LibvirtXmlSerializationException | LibvirtXmlValidationException e ) {
 			throw new VirtualizationConfigurationException( e.getLocalizedMessage() );
 		}
@@ -239,7 +239,7 @@ public class VirtualizationConfigurationQemu extends VirtualizationConfiguration
 	@Override
 	public boolean addEmptyHddTemplate()
 	{
-		return this.addHddTemplate( new String(), null, null );
+		return this.addHddTemplate( "", null, null );
 	}
 
 	@Override
diff --git a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemuUtils.java b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemuUtils.java
index 1befdc4..7b1b9bb 100644
--- a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemuUtils.java
+++ b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemuUtils.java
@@ -114,7 +114,7 @@ public class VirtualizationConfigurationQemuUtils
 	private static String createAlphabeticalDeviceName( String devicePrefix, int deviceNumber )
 	{
 		if ( deviceNumber < 0 || deviceNumber >= ( 'z' - 'a' ) ) {
-			String errorMsg = new String( "Device number is out of range to be able to create a valid device name." );
+			String errorMsg = "Device number is out of range to be able to create a valid device name.";
 			throw new IllegalArgumentException( errorMsg );
 		}
 
diff --git a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualboxFileFormat.java b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualboxFileFormat.java
index b4cb7c3..af7c54c 100644
--- a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualboxFileFormat.java
+++ b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualboxFileFormat.java
@@ -44,7 +44,7 @@ public class VirtualizationConfigurationVirtualboxFileFormat
 	private static final Logger LOGGER = LogManager.getLogger( VirtualizationConfigurationVirtualboxFileFormat.class );
 
 	// key information set during initial parsing of the XML file
-	private String osName = new String();
+	private String osName = "";
 	private ArrayList<HardDisk> hddsArray = new ArrayList<HardDisk>();
 
 	// XPath and DOM parsing related members
diff --git a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVmwareFileFormat.java b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVmwareFileFormat.java
index 134ff30..356a034 100644
--- a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVmwareFileFormat.java
+++ b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVmwareFileFormat.java
@@ -117,12 +117,22 @@ public class VirtualizationConfigurationVmwareFileFormat
 	public static BufferedReader getVmxReader( byte[] vmxContent, int length ) throws IOException
 	{
 		Charset cs = getCharset( vmxContent, length );
+		if ( cs == null )
+			cs = StandardCharsets.UTF_8; // YES BECAUSE THIS IS NOT VMX AND EVERYTHING IS SHIT
 		return new BufferedReader( new InputStreamReader( new ByteArrayInputStream( vmxContent, 0, length ), cs ) );
 	}
 
+	/**
+	 * Get charset of config. Returns null if input doesn't look like a vmx file.
+	 * @param vmxContent
+	 * @param length
+	 * @return
+	 */
 	public static Charset getCharset( byte[] vmxContent, int length )
 	{
 		String csName = detectCharset( new ByteArrayInputStream( vmxContent, 0, length ) );
+		if ( csName == null )
+			return null;
 		Charset cs = null;
 		try {
 			cs = Charset.forName( csName );
@@ -146,8 +156,9 @@ public class VirtualizationConfigurationVmwareFileFormat
 		return ret;
 	}
 
-	public static String detectCharset( InputStream is )
+	private static String detectCharset( InputStream is )
 	{
+		boolean isVmware = false;
 		try {
 			BufferedReader csDetectReader = new BufferedReader( new InputStreamReader( is, StandardCharsets.ISO_8859_1 ) );
 			String line;
@@ -158,10 +169,15 @@ public class VirtualizationConfigurationVmwareFileFormat
 				if ( entry.key.equals( ".encoding" ) || entry.key.equals( "encoding" ) ) {
 					return entry.value;
 				}
+				if ( entry.key.equals( "virtualHW.version" ) || entry.key.equals( "memsize" ) || entry.key.equals( "displayName") ) {
+					isVmware = true;
+				}
 			}
 		} catch ( Exception e ) {
 			LOGGER.warn( "Could not detect charset, fallback to latin1", e );
 		}
+		if ( !isVmware )
+			return null;
 		// Dumb fallback
 		return "ISO-8859-1";
 	}
diff --git a/src/main/java/org/openslx/virtualization/configuration/transformation/TransformationManager.java b/src/main/java/org/openslx/virtualization/configuration/transformation/TransformationManager.java
index 6bb5faa..18d7e43 100644
--- a/src/main/java/org/openslx/virtualization/configuration/transformation/TransformationManager.java
+++ b/src/main/java/org/openslx/virtualization/configuration/transformation/TransformationManager.java
@@ -122,8 +122,8 @@ public class TransformationManager<T, R>
 			try {
 				transformation.apply( this.config, this.args );
 			} catch ( TransformationException e ) {
-				final String errorMsg = new String(
-						"Error in configuration filter '" + transformation.getName() + "': " + e.getLocalizedMessage() );
+				final String errorMsg =
+						"Error in configuration filter '" + transformation.getName() + "': " + e.getLocalizedMessage();
 				throw new TransformationException( errorMsg );
 			}
 		}
@@ -136,7 +136,7 @@ public class TransformationManager<T, R>
 	 */
 	private String showTransformations()
 	{
-		String transformationSummary = new String();
+		String transformationSummary = "";
 		final int maxFilterNumCharacters = ( this.transformations.size() + 1 ) / 10;
 
 		for ( int i = 0; i < this.transformations.size(); i++ ) {
diff --git a/src/main/java/org/openslx/virtualization/disk/DiskImage.java b/src/main/java/org/openslx/virtualization/disk/DiskImage.java
index 5271e6e..dfb242a 100644
--- a/src/main/java/org/openslx/virtualization/disk/DiskImage.java
+++ b/src/main/java/org/openslx/virtualization/disk/DiskImage.java
@@ -136,7 +136,7 @@ public abstract class DiskImage implements Closeable
 			throw e;
 		}
 		Util.safeClose( fileHandle );
-		final String errorMsg = new String( "File '" + diskImagePath.getAbsolutePath() + "' is not a valid disk image!" );
+		final String errorMsg = "File '" + diskImagePath.getAbsolutePath() + "' is not a valid disk image!";
 		throw new DiskImageException( errorMsg );
 	}
 
diff --git a/src/main/java/org/openslx/virtualization/disk/DiskImageQcow2.java b/src/main/java/org/openslx/virtualization/disk/DiskImageQcow2.java
index a007e1c..a6b3b73 100644
--- a/src/main/java/org/openslx/virtualization/disk/DiskImageQcow2.java
+++ b/src/main/java/org/openslx/virtualization/disk/DiskImageQcow2.java
@@ -210,7 +210,7 @@ public class DiskImageQcow2 extends DiskImage
 		// check QCOW2 file format version
 		if ( qcowVersion < 2 || qcowVersion > 3 ) {
 			// QCOW2 disk image does not contain a valid QCOW2 version
-			final String errorMsg = new String( "Invalid QCOW2 version in header found!" );
+			final String errorMsg = "Invalid QCOW2 version in header found!";
 			throw new DiskImageException( errorMsg );
 		}
 
diff --git a/src/main/java/org/openslx/virtualization/disk/DiskImageUtils.java b/src/main/java/org/openslx/virtualization/disk/DiskImageUtils.java
index dc67548..6bcef7f 100644
--- a/src/main/java/org/openslx/virtualization/disk/DiskImageUtils.java
+++ b/src/main/java/org/openslx/virtualization/disk/DiskImageUtils.java
@@ -119,11 +119,11 @@ public class DiskImageUtils
 	 * @param diskImage file to a disk storing the image content.
 	 * @param offset offset in bytes for reading <code>numBytes</code> bytes.
 	 * @param numBytes number of bytes to read at <code>offset</code>.
-	 * @return read bytes from the disk image file as {@link String}.
+	 * @return read bytes from the disk image file as {@link byte[]}.
 	 * 
-	 * @throws DiskImageException unable to read two bytes from the disk image file.
+	 * @throws DiskImageException unable to read bytes from the disk image file.
 	 */
-	public static String readBytesAsString( RandomAccessFile diskImage, long offset, int numBytes )
+	public static byte[] readBytesAsArray( RandomAccessFile diskImage, long offset, int numBytes )
 			throws DiskImageException
 	{
 		final long imageSize = DiskImageUtils.getImageSize( diskImage );
@@ -139,6 +139,6 @@ public class DiskImageUtils
 			}
 		}
 
-		return new String( values );
+		return values;
 	}
 }
diff --git a/src/main/java/org/openslx/virtualization/disk/DiskImageVdi.java b/src/main/java/org/openslx/virtualization/disk/DiskImageVdi.java
index c564112..1e5b20b 100644
--- a/src/main/java/org/openslx/virtualization/disk/DiskImageVdi.java
+++ b/src/main/java/org/openslx/virtualization/disk/DiskImageVdi.java
@@ -1,6 +1,8 @@
 package org.openslx.virtualization.disk;
 
 import java.io.RandomAccessFile;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
 
 import org.openslx.virtualization.Version;
 
@@ -16,6 +18,10 @@ public class DiskImageVdi extends DiskImage
 	 * Big endian representation of the little endian VDI magic bytes (signature).
 	 */
 	private static final int VDI_MAGIC = 0x7f10dabe;
+	/**
+	 * Just 16 null-bytes
+	 */
+	private static final byte[] ZERO_UUID = new byte[16];
 
 	/**
 	 * Creates a new VDI disk image from an existing VDI image file.
@@ -73,10 +79,9 @@ public class DiskImageVdi extends DiskImage
 		final RandomAccessFile diskFile = this.getDiskImage();
 
 		// if parent UUID is set, the VDI file is a snapshot
-		final String parentUuid = DiskImageUtils.readBytesAsString( diskFile, 440, 16 );
-		final String zeroUuid = new String( new byte[ 16 ] );
+		byte[] parentUuid = DiskImageUtils.readBytesAsArray( diskFile, 440, 16 );
 
-		return !zeroUuid.equals( parentUuid );
+		return !Arrays.equals( ZERO_UUID, parentUuid );
 	}
 
 	@Override
@@ -94,7 +99,9 @@ public class DiskImageVdi extends DiskImage
 	public String getDescription() throws DiskImageException
 	{
 		final RandomAccessFile diskFile = this.getDiskImage();
-		return DiskImageUtils.readBytesAsString( diskFile, 84, 256 );
+		byte[] data = DiskImageUtils.readBytesAsArray( diskFile, 84, 256 );
+		// This will replace invalid chars. Maybe use CharsetDecoder and fall back to latin1 on error
+		return new String( data, StandardCharsets.UTF_8 );
 	}
 
 	@Override
diff --git a/src/main/java/org/openslx/virtualization/disk/DiskImageVmdk.java b/src/main/java/org/openslx/virtualization/disk/DiskImageVmdk.java
index 720ec37..77986ef 100644
--- a/src/main/java/org/openslx/virtualization/disk/DiskImageVmdk.java
+++ b/src/main/java/org/openslx/virtualization/disk/DiskImageVmdk.java
@@ -1,6 +1,7 @@
 package org.openslx.virtualization.disk;
 
 import java.io.RandomAccessFile;
+import java.nio.charset.StandardCharsets;
 
 import org.openslx.util.Util;
 import org.openslx.virtualization.configuration.VirtualizationConfigurationVmwareFileFormat;
@@ -117,15 +118,15 @@ public class DiskImageVmdk extends DiskImage
 			// get content of descriptor file embedded into the VMDK disk image
 			final long vmdkDescriptorOffset = vmdkDescriptorSectorOffset * DiskImageVmdk.VMDK_SECTOR_SIZE;
 			final long vmdkDescriptorSizeMax = vmdkDescriptorSectorSize * DiskImageVmdk.VMDK_SECTOR_SIZE;
-			final String descriptorStr = DiskImageUtils.readBytesAsString( diskFile, vmdkDescriptorOffset,
-					Long.valueOf( vmdkDescriptorSizeMax ).intValue() );
+			final String descriptorStr = new String ( DiskImageUtils.readBytesAsArray( diskFile, vmdkDescriptorOffset,
+					Long.valueOf( vmdkDescriptorSizeMax ).intValue() ), StandardCharsets.US_ASCII );
 
 			// get final length of the content within the sectors to be able to trim all 'zero' characters
 			final int vmdkDescriptorSize = descriptorStr.indexOf( 0 );
 
 			// if final length of the content is invalid, throw an exception 
 			if ( vmdkDescriptorSize > vmdkDescriptorSizeMax || vmdkDescriptorSize < 0 ) {
-				final String errorMsg = new String( "Embedded descriptor size in VMDK disk image is invalid!" );
+				final String errorMsg = "Embedded descriptor size in VMDK disk image is invalid!";
 				throw new DiskImageException( errorMsg );
 			}