Changeset 3600

Timestamp:

Jul 25, 2007, 1:14:48 PM (16 years ago)

Author:

Nicklas Nordborg

Message:

Fixes #480: Verify values in extended-properties.xml and raw-data-types.xml

Location:

trunk

Files:

• doc/src/docbook/appendix/incompatible.xml (modified) (1 diff)
• src/core/net/sf/basedb/core/Application.java (modified) (1 diff)
• src/core/net/sf/basedb/core/ExtendedProperties.java (modified) (3 diffs)
• src/core/net/sf/basedb/core/ExtendedProperty.java (modified) (2 diffs)
• src/core/net/sf/basedb/core/HibernateUtil.java (modified) (5 diffs)
• src/core/net/sf/basedb/core/RawDataTypes.java (modified) (5 diffs)
• src/core/net/sf/basedb/core/data/ExtendableData.java (modified) (2 diffs)
• src/core/net/sf/basedb/core/dbengine/AbstractDbEngine.java (modified) (3 diffs)
• src/core/net/sf/basedb/core/dbengine/DbEngine.java (modified) (1 diff)

trunk/doc/src/docbook/appendix/incompatible.xml

@@ -79 +79 @@
       old behaviour use <code>ch(1) == 'NULL'</code>.
     </para>
+    
+    <bridgehead>Extended properties and raw data types</bridgehead>
+    <para>
+      We have added validation code to check for invalid values. If you
+      have modified the <filename>extended-properties.xml</filename>
+      or the <filename>raw-data-types.xml</filename> file and they
+      contain invalid values, you may not be able to start BASE until 
+      they are fixed. The validation is rather strict and things that may
+      have worked before (because you were lucky or the because the database
+      has been forgiving) may no longer work. Here is an overview of the most
+      important validation rules:
+    </para>
+    
+    <itemizedlist>
+    <listitem>
+      <para>
+      Names and identifiers for extended properties and raw data type
+      can only contain letters, digits and underscores. They must not
+      start with a digit.
+      </para>
+    </listitem>
+    <listitem>
+      <para>
+      Names of database tables and columns can only contain letters, 
+      digits and underscores. They must not start with a digit.
+      </para>
+    </listitem>
+    <listitem>
+      <para>
+      There mustn't be any duplicate tables, columns, properties, etc.
+      for a given context. For example, no duplicate tables in the
+      database, no duplicate columns in a table, and no duplicate
+      properties for a raw data type.
+      </para>
+    </listitem>
+    </itemizedlist>
+    
   </sect1>
   

trunk/src/core/net/sf/basedb/core/Application.java

@@ -392 +392 @@
       
       // Initialise other utility classes
+      HibernateUtil.init1();
       ExtendedProperties.init();
       RawDataTypes.init();
       RawDataUtil.init();
-      HibernateUtil.init();
+      HibernateUtil.init2();
       QueryRuntimeFilterFactory.init();
       PredefinedQuery.init();

trunk/src/core/net/sf/basedb/core/ExtendedProperties.java

@@ -24 +24 @@
 package net.sf.basedb.core;
 
+import net.sf.basedb.core.dbengine.DbEngine;
+import net.sf.basedb.util.Values;
 import net.sf.basedb.util.XMLUtil;
 
+import java.util.HashSet;
 import java.util.List;
 import java.util.ArrayList;
 import java.util.Map;
 import java.util.HashMap;
+import java.util.Set;
+import java.util.regex.PatternSyntaxException;
 import java.net.URL;
 
@@ -194 +199 @@
     List<ExtendedProperty> properties = new ArrayList<ExtendedProperty>();
     List<Element> children = (List<Element>)classElement.getChildren("property");
+    DbEngine engine = HibernateUtil.getDbEngine();
+    String className = classElement.getAttributeValue("name");
+    Set<String> usedNames = new HashSet<String>();
     for (Element property : children)
     {
-      String name = property.getAttributeValue("name");
-      String title = property.getAttributeValue("title");
+      String name = Values.getStringOrNull(property.getAttributeValue("name"));
+      if (!ExtendedProperty.isValidName(name))
+      {
+        throw new InvalidDataException("Invalid property for class " + 
+          className + ": name=" + name);
+      }
+      if (usedNames.contains("name:" + name))
+      {
+        throw new InvalidDataException("Duplicate property for class " + 
+          className + ": name=" + name);
+      }
+      usedNames.add("name:" + name);
+      String title = Values.getStringOrNull(property.getAttributeValue("title"));
       if (title == null) title = name;
+      String column = Values.getStringOrNull(property.getAttributeValue("column"));
+      if (!engine.isValidColumnName(column))
+      {
+        throw new InvalidDataException("Invalid column for property " + 
+          className + "[" + name + "]: column=" + column);
+      }
+      if (usedNames.contains("column:" + column))
+      {
+        throw new InvalidDataException("Duplicate column for property " + 
+          className + "[" + name + "]: column=" + column);
+      }
+      usedNames.add("column:" + column);
+      String description = Values.getStringOrNull(property.getAttributeValue("description"));
+      int length = XMLUtil.getIntAttribute(property, "length", 255);
       Type type = Type.fromValue(property.getAttributeValue("type"));
-      String column = property.getAttributeValue("column");
-      String description = property.getAttributeValue("description");
-      int length = XMLUtil.getIntAttribute(property, "length", 255);
       if (type == Type.STRING && length > 255) type = Type.TEXT;
       boolean nullable = XMLUtil.getBooleanAttribute(property, "null", true);
@@ -230 +260 @@
         for (Element link : links)
         {
-          String regexp = link.getAttributeValue("regexp");
-          String url = link.getAttributeValue("url");
-          epLinks.add(new ExtendedPropertyLinker(regexp, url));
+          String regexp = Values.getStringOrNull(link.getAttributeValue("regexp"));
+          String url = Values.getStringOrNull(link.getAttributeValue("url"));
+          if (url == null)
+          {
+            throw new InvalidDataException("Missing url for property link " + 
+              className + "[" + name + "]: regexp=" + regexp);
+          }
+          try
+          {
+            epLinks.add(new ExtendedPropertyLinker(regexp, url));
+          }
+          catch (PatternSyntaxException ex)
+          {
+            throw new InvalidDataException("Invalid regexp for property link " + 
+              className + "[" + name + "]: regexp=" + regexp, ex);
+          }
         }
       }

trunk/src/core/net/sf/basedb/core/ExtendedProperty.java

@@ -26 +26 @@
 import java.text.NumberFormat;
 import java.util.List;
+import java.util.regex.Pattern;
+
 
 /**
@@ -37 +39 @@
 public class ExtendedProperty
 {
+  
+  /**
+    A regexp checking for invalid characters.
+  */
+  private static final Pattern valid = Pattern.compile("[a-zA-Z_][a-zA-Z0-9_]*");
+
+  /**
+    Check that the name only contains a-zA-Z0-9_ and starts with
+    a letter or underscore.
+    @since 2.4
+  */
+  public static boolean isValidName(String name)
+  {
+    return name == null ? false : valid.matcher(name).matches();
+  }
+  
   private final String name;
   private final Type type;

trunk/src/core/net/sf/basedb/core/HibernateUtil.java

@@ -138 +138 @@
 
   /**
-    Initialise this class. This is done at startup time by the
-    {@link Application#start()} method. Initialising means that we
+    First step of initialising this class. This is done at startup time by the
+    {@link Application#start()} method. In this step we load configuration
+    settings from the 'base.config' and 'hibernate.cfg.xml' files and
+    create the Dialect and DbEngine objects.
+    
+    Initialising means that we
     load the configuration from the properties and the xml file,
     read all mapping files, generate additional mappings for the
@@ -145 +149 @@
     used by {@link Query} implementation.
   */
-  static synchronized void init()
+  static synchronized void init1()
     throws BaseException
   {
@@ -155 +159 @@
       cfg = new Configuration();
       setConfigurationProperties(cfg);
+      dialect = Dialect.getDialect(cfg.getProperties());
+      dbEngine = EngineFactory.createEngine(dialect);
+    }
+    catch (HibernateException ex)
+    {
+      throw new BaseException(ex);
+    }
+  }
+
+  /**
+    Second step of initialising this class. This is done at startup time by the
+    {@link Application#start()} method. In this step we read all mapping files, 
+    generate additional mappings for the {@link ExtendableData} items and raw data, 
+    and generate filters  used by {@link Query} implementation.
+  */
+  static synchronized void init2()
+    throws BaseException
+  {
+    // Return if we have already been initialised
+    if (isInitialised) return;
+  
+    try
+    {
       addStaticMappings(cfg);
       addExtendedPropertiesMappings(cfg);
@@ -161 +188 @@
       cfg.configure();
       sf = cfg.buildSessionFactory();
-      dialect = Dialect.getDialect(cfg.getProperties());
-      dbEngine = EngineFactory.createEngine(dialect);
     }
     catch (HibernateException ex)
@@ -170 +195 @@
     isInitialised = true;
   }
-
+  
   /**
     Unload all settings.

trunk/src/core/net/sf/basedb/core/RawDataTypes.java

@@ -24 +24 @@
 package net.sf.basedb.core;
 
+import net.sf.basedb.core.dbengine.DbEngine;
+import net.sf.basedb.util.Values;
 import net.sf.basedb.util.XMLUtil;
 
+import java.util.HashSet;
 import java.util.List;
 import java.util.ArrayList;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeMap;
 import java.net.URL;
@@ -141 +145 @@
   {
     List<Element> rawDataTypeTags = dom.getRootElement().getChildren("raw-data-type");
+    DbEngine engine = HibernateUtil.getDbEngine();
+    Set<String> usedNames = new HashSet<String>();
     for (Element el : rawDataTypeTags)
     {
-      String id = el.getAttributeValue("id");
-      String name = el.getAttributeValue("name");
-      String description = el.getAttributeValue("description");
+      String id = Values.getStringOrNull(el.getAttributeValue("id"));
+      String name = Values.getStringOrNull(el.getAttributeValue("name"));
+      String description = Values.getStringOrNull(el.getAttributeValue("description"));
       String storage = el.getAttributeValue("storage");
-      String table = el.getAttributeValue("table");
+      String table = Values.getStringOrNull(el.getAttributeValue("table"));
       int channels = XMLUtil.getIntAttribute(el, "channels", 2);
-      List<RawDataProperty> properties = loadProperties(el);
+      
+      if (!ExtendedProperty.isValidName(id))
+      {
+        throw new InvalidDataException("Invalid id for raw data type: " + id);
+      }
+      if (usedNames.contains("name:" + name))
+      {
+        throw new InvalidDataException("Duplicate name for raw data type " + 
+          id +": name=" + name);
+      }
+      usedNames.add("name:" + name);
+      if ("database".equals(storage))
+      {
+        if (!engine.isValidTableName(table))
+        {
+          throw new InvalidDataException("Invalid table for raw data type " + 
+              id +": table=" + table);
+        }
+        if (usedNames.contains("table:" + table))
+        {
+          throw new InvalidDataException("Duplicate table for raw data type " + 
+            id +": table=" + table);
+        }
+        usedNames.add("table:" + table);
+      }
+      if (channels <= 0)
+      {
+        throw new InvalidDataException("Number of channels must be > 0 for raw data type "+
+            id + ": channels=" + channels);
+      }
+      List<RawDataProperty> properties = loadProperties(el, channels);
       List<IntensityFormula> formulas = loadIntensityFormulas(el, channels);
       RawDataType rdt = new RawDataType(id, name, description, channels, storage, table, properties, formulas);
@@ -161 +197 @@
   */
   @SuppressWarnings({"unchecked"})
-  private static List<RawDataProperty> loadProperties(Element rawDataTypeElement)
+  private static List<RawDataProperty> loadProperties(Element rawDataTypeElement, int channels)
   {
     List<RawDataProperty> properties = new ArrayList<RawDataProperty>();
     List<Element> children = rawDataTypeElement.getChildren("property");
+    String rawDataType = rawDataTypeElement.getAttributeValue("id");
+    DbEngine engine = HibernateUtil.getDbEngine();
+    Set<String> usedNames = new HashSet<String>();
     for (Element property : children)
     {
-      String name = property.getAttributeValue("name");
-      String title = property.getAttributeValue("title");
+      String name = Values.getStringOrNull(property.getAttributeValue("name"));
+      if (!ExtendedProperty.isValidName(name))
+      {
+        throw new InvalidDataException("Invalid property for raw data type " + 
+          rawDataType + ": name=" + name);
+      }
+      if (usedNames.contains("name:" + name))
+      {
+        throw new InvalidDataException("Duplicate property for raw data type " + 
+          rawDataType + ": name=" + name);
+      }
+      usedNames.add("name:" + name);
+      String title = Values.getStringOrNull(property.getAttributeValue("title"));
       if (title == null) title = name;
-      String column = property.getAttributeValue("column");
-      String description = property.getAttributeValue("description");
+      String column = Values.getStringOrNull(property.getAttributeValue("column"));
+      if (!engine.isValidColumnName(column))
+      {
+        throw new InvalidDataException("Invalid column for property " + 
+          rawDataType + "[" + name + "]: column=" + column);
+      }
+      if (usedNames.contains("column:" + column))
+      {
+        throw new InvalidDataException("Duplicate column for property " + 
+          rawDataType + "[" + name + "]: column=" + column);
+      }
+      usedNames.add("column:" + column);
+      String description = Values.getStringOrNull(property.getAttributeValue("description"));
       Type type = Type.fromValue(property.getAttributeValue("type"));
       int length = XMLUtil.getIntAttribute(property, "length", 255);
@@ -192 +253 @@
       }
       int channel = XMLUtil.getIntAttribute(property, "channel", 0);
+      if (channel < 0 || channel > channels)
+      {
+        throw new InvalidDataException("Channel for property " + rawDataType+ "[" + name + "]" 
+            +  " must be >= 0 and < " + channels + ": channel=" + channel);
+      }
       properties.add(new RawDataProperty(name, title, description, column, type, length, nullable, averageMethod, channel));
     }
@@ -206 +272 @@
     List<IntensityFormula> formulas = new ArrayList<IntensityFormula>();
     List<Element> children = rawDataTypeElement.getChildren("intensity-formula");
+    String rawDataType = rawDataTypeElement.getAttributeValue("id");
+    Set<String> usedNames = new HashSet<String>();
     for (Element formula : children)
     {
-      String name = formula.getAttributeValue("name");
-      String title = formula.getAttributeValue("title");
+      String name = Values.getStringOrNull(formula.getAttributeValue("name"));
+      if (!ExtendedProperty.isValidName(name))
+      {
+        throw new InvalidDataException("Invalid intensity formula for raw data type " + 
+          rawDataType + ": name=" + name);
+      }
+      if (usedNames.contains("name:" + name))
+      {
+        throw new InvalidDataException("Duplicate intensity formula for raw data type " + 
+          rawDataType + ": name=" + name);
+      }
+      usedNames.add("name:" + name);
+      String title = Values.getStringOrNull(formula.getAttributeValue("title"));
       if (title == null) title = name;
-      String description = formula.getAttributeValue("description");
+      String description = Values.getStringOrNull(formula.getAttributeValue("description"));
       String[] expressions = new String[channels];
       for (Element expression :  (List<Element>)formula.getChildren("formula"))
       {
         int channel = XMLUtil.getIntAttribute(expression, "channel", 0);
+        String exp = Values.getStringOrNull(expression.getAttributeValue("expression"));
         if (channel <= 0 || channel > channels)
         {
-          throw new BaseException("Invalid channel number for expression: "+expression);
+          throw new InvalidDataException("Channel for intensity formula " + 
+              rawDataType+ "[" + name + "]" 
+              +  " must be > 0 and < " + channels + ": channel=" + channel);
         }
         if (expressions[channel-1] != null)
         {
-          throw new BaseException("Expression for channel "+channel+" has already been defined: " +expression);
-        }
-        expressions[channel-1] = expression.getAttributeValue("expression");
+          throw new InvalidDataException("Duplicate expression for intensity formula " + 
+            rawDataType + "[" + name + "]: channel=" + channel);
+        }
+        if (exp == null)
+        {
+          throw new InvalidDataException("Missing expression for formula " + 
+            rawDataType + "[" + name + "]: channel=" + channel);
+        }
+        expressions[channel-1] = exp;
       }
       formulas.add(new IntensityFormula(name, title, description, expressions));

trunk/src/core/net/sf/basedb/core/data/ExtendableData.java

@@ -68 +68 @@
     <td>
       The property name of the extended property.
-      See {@link net.sf.basedb.core.ExtendedProperty#getName()}.
+      See {@link net.sf.basedb.core.ExtendedProperty#getName()}. The name
+      must only contain letters, numbers and underscores but the first character
+      can't be a number. The name must be unique within the class.
     </td>
   </tr>
@@ -89 +91 @@
     <td>
       The database column name of the extended property. This value
-      must of course be unique for each class.
-      See {@link net.sf.basedb.core.ExtendedProperty#getColumn()}.
+      must be unique for each class. The value is validated by the 
+      {@link net.sf.basedb.core.dbengine.DbEngine#isValidColumnName(String)}
+      which normally means it must follow the same rules as the <code>name</code>
+      attribute. See {@link net.sf.basedb.core.ExtendedProperty#getColumn()}.
     </td>
   </tr>

trunk/src/core/net/sf/basedb/core/dbengine/AbstractDbEngine.java

@@ -24 +24 @@
 package net.sf.basedb.core.dbengine;
 
+import java.util.regex.Pattern;
+
 /**
   An abstract superclass with default implementations for most {@link DbEngine}
@@ -36 +38 @@
 {
 
+  /**
+    A regexp checking for invalid characters.
+  */
+  private static final Pattern valid = Pattern.compile("[a-zA-Z_][a-zA-Z0-9_]*");
+  
   /**
     Create AbstractDbEngine.
@@ -121 +128 @@
     return "EXP("+ value + ")";
   }
+  /**
+    Checks that the name only contains the following characters: a-zA-Z0-9_
+    It must also start with a letter or underscore.
+    @since 2.4
+  */
+  public boolean isValidTableName(String tableName)
+  {
+    return isValidName(tableName);
+  }
+  
+  /**
+    Checks that the name only contains the following characters: a-zA-Z0-9_
+    It must also start with a letter or underscore.
+  */
+  public boolean isValidColumnName(String columnName)
+  {
+    return isValidName(columnName);
+  }
   // -------------------------------------------
 
+  /**
+    Check that the name only contains a-zA-Z0-9_ and starts with
+    a letter or underscore.
+    @since 2.4
+  */
+  protected boolean isValidName(String name)
+  {
+    return name == null ? false : valid.matcher(name).matches();
+  }
+  
+  
 }

trunk/src/core/net/sf/basedb/core/dbengine/DbEngine.java

@@ -239 +239 @@
   public String exp(String value);
 
+  /**
+    Check if a given string is valid to be used as a table name in the
+    current database.
+    @param tableName The string to check
+    @return TRUE if the name is valid, FALSE if not
+    @since 2.4
+  */
+  public boolean isValidTableName(String tableName);
+  
+  /**
+    Check if a given string is valid to be used as a column name in the
+    current database.
+    @param columnName The string to check
+    @return TRUE if the name is valid, FALSE if not
+    @since 2.4
+  */
+  public boolean isValidColumnName(String columnName);
 }