added "blacklist" of attachment MIME types that won't be compressed; a couple

of other minor tweaks

etc/logging-config.xml

@@ -20,11 +20,13 @@
 <configuration>
 
   <!-- Define the standard file appender. -->
-  <appender name="STDLOG" class="org.apache.log4j.FileAppender">
+  <appender name="STDLOG" class="org.apache.log4j.RollingFileAppender">
     <param name="File" value="/home/erbo/venice.log"/>
-    <param name="Append" value="false"/>
+    <param name="Append" value="true"/>
+    <param name="MaxFileSize" value="10MB"/>
+    <param name="MaxBackupIndex" value="5"/>
     <layout class="org.apache.log4j.PatternLayout">
-      <param name="ConversionPattern" value="%d %-5p %c{2} %x - %m%n"/>
+      <param name="ConversionPattern" value="%d %-5p %c{2} [%t %x] - %m%n"/>
     </layout>
   </appender>
 
@@ -36,7 +38,7 @@
 
   <!-- Turn down the standard detail in some areas -->
   <category name="com.silverwrist.util.ServletMultipartHandler">
-    <priority value="fatal"/>
+    <priority value="debug"/>
   </category>
   <category name="com.silverwrist.venice.htmlcheck">
     <priority value="fatal"/>

etc/venice-config.xml

@@ -72,6 +72,17 @@
     <file>/home/erbo/venice/WEB-INF/erbo.dict</file>
   </dictionary>
 
+  <!-- Settings for dealing with uploads -->
+  <upload>
+    <!-- Don't try to compress any file whose type falls in this list -->
+    <no-compress>
+      <type>image/gif</type>
+      <type>image/jpg</type>
+      <type>image/jpeg</type>
+    </no-compress>
+
+  </upload>
+
   <!-- This section holds "stock messages" with replaceable parameters that may
        be fed to emailed output. -->
   <messages>

src/com/silverwrist/util/ServletMultipartHandler.java

@@ -342,7 +342,7 @@ public class ServletMultipartHandler
    *--------------------------------------------------------------------------------
    */
 
-  private static Category logger = Category.getInstance(ServletMultipartHandler.class.getName());
+  private static Category logger = Category.getInstance(ServletMultipartHandler.class);
 
   /*--------------------------------------------------------------------------------
    * Attributes
@@ -350,8 +350,8 @@ public class ServletMultipartHandler
    */
 
   private MimeMultipart multipart;     // holds all the multipart data
-  private Hashtable param_byname;      // parameters by name
+  private HashMap param_byname;        // parameters by name
-  private Vector param_order;          // parameters in order
+  private ArrayList param_order;       // parameters in order
 
   /*--------------------------------------------------------------------------------
    * Constructor
@@ -360,34 +360,29 @@ public class ServletMultipartHandler
 
   public ServletMultipartHandler(ServletRequest request) throws ServletMultipartException
   {
-    if (logger.isDebugEnabled())
-      logger.debug("creating new ServletMultipartHandler");
-
-    if (!canHandle(request))
-    { // we can't handle ordinary requests, just multipart/form-data ones
-      logger.error("this request is not of type multipart/form-data");
-      throw new ServletMultipartException("not a multipart/form-data request");
-
-    } // end if
-
     try
     { // build the MimeMultipart based on the ServletDataSource
+      logger.debug("about to create MimeMultipart");
       multipart = new MimeMultipart(new ServletDataSource(request));
+      logger.debug("about to get request count");
       int count = multipart.getCount();
       if (logger.isDebugEnabled())
 	logger.debug("retrieved " + count + " parameter(s) from request");
       
       // allocate the multipart parameters and slot them into our arrays
-      param_byname = new Hashtable();
+      param_byname = new HashMap();
-      param_order = new Vector();
+      param_order = new ArrayList();
       for (int i=0; i<count; i++)
       { // create the MultipartParameter structures and stash them for later use
 	MultipartParameter parm = new MultipartParameter((MimeBodyPart)(multipart.getBodyPart(i)));
 	param_byname.put(parm.getName(),parm);
-	param_order.addElement(parm);
+	param_order.add(parm);
 
       } // end count
 
+      if (logger.isDebugEnabled())
+	logger.debug("ServletMultipartHandler created, " + param_order.size() + " params defined");
+
     } // end try
     catch (MessagingException me)
     { // translate a MessagingException into the nicer ServletMultipartException
@@ -425,19 +420,34 @@ public class ServletMultipartHandler
 
   public Enumeration getNames()
   {
-    Vector tmp_vector = new Vector();
+    ArrayList tmp_v = new ArrayList();
-    Enumeration enum = param_order.elements();
+    Iterator it = param_order.iterator();
-    while (enum.hasMoreElements())
+    while (it.hasNext())
     { // add each name to the temporary vector
-      MultipartParameter parm = (MultipartParameter)(enum.nextElement());
+      MultipartParameter parm = (MultipartParameter)(it.next());
-      tmp_vector.addElement(parm.getName());
+      tmp_v.add(parm.getName());
 
     } // end while
 
-    return tmp_vector.elements(); // and enumerate it
+    return Collections.enumeration(tmp_v); // and enumerate it
 
   } // end getNames
 
+  public Iterator getParamNames()
+  {
+    ArrayList tmp_v = new ArrayList();
+    Iterator it = param_order.iterator();
+    while (it.hasNext())
+    { // add each name to the temporary vector
+      MultipartParameter parm = (MultipartParameter)(it.next());
+      tmp_v.add(parm.getName());
+
+    } // end while
+
+    return Collections.unmodifiableList(tmp_v).iterator();
+    
+  } // end getParamNames
+
   public boolean isFileParam(String name)
   {
     MultipartParameter parm = (MultipartParameter)(param_byname.get(name));

src/com/silverwrist/util/test/FormDataTest.java

@@ -14,6 +14,7 @@ public class FormDataTest extends HttpServlet
   public void doGet(HttpServletRequest request, HttpServletResponse response)
     throws IOException, ServletException
   {
+    response.setContentType("text/html");
     PrintWriter out = response.getWriter();
     out.println("<HTML><HEAD><TITLE>Form Test</TITLE></HEAD><BODY>");
     out.println("<H1>Form Test</H1>");
@@ -103,6 +104,7 @@ public class FormDataTest extends HttpServlet
 
     } // end catch
 
+    response.setContentType("text/html");
     PrintWriter out = response.getWriter();
     out.println("<HTML><HEAD><TITLE>Form Test Results</TITLE></HEAD><BODY>");
     out.println("<H1>Form Test Results</H1>");

src/com/silverwrist/venice/core/impl/EngineBackend.java

@@ -97,4 +97,6 @@ public interface EngineBackend
 
   public abstract boolean isValidRandomAuthString(String s);
 
+  public abstract boolean isNoCompressMimeType(String type);
+
 } // end interface EngineBackend

src/com/silverwrist/venice/core/impl/TopicMessageUserContextImpl.java

@@ -35,7 +35,7 @@ class TopicMessageUserContextImpl implements TopicMessageContext
    *--------------------------------------------------------------------------------
    */
 
-  private static Category logger = Category.getInstance(TopicMessageUserContextImpl.class.getName());
+  private static Category logger = Category.getInstance(TopicMessageUserContextImpl.class);
 
   private static final int MAX_ATTACH = 1048576;  // TODO: should be a configurable parameter
 
@@ -813,6 +813,9 @@ class TopicMessageUserContextImpl implements TopicMessageContext
   public void attachData(String m_type, String file, int length, InputStream data)
       throws AccessError, DataException
   {
+    if (logger.isDebugEnabled())
+      logger.debug("attachData to msg " + postid + ": (" + m_type + ", " + file + ", " + length + ")");
+
     if (mimetype!=null)
     { // the message already has an attachment
       logger.error("tried to attach data to a message that already has it!");
@@ -865,6 +868,20 @@ class TopicMessageUserContextImpl implements TopicMessageContext
     // Compress the attachment data in memory.
     InputStream real_data = null;
     int real_length = 0;
+    int tmp_stgmethod;
+
+    if (engine.isNoCompressMimeType(m_type))
+    { // don't compress me, just store me normally
+      logger.debug("stored as NORMAL data (no compression)");
+      real_data = data;
+      real_length = length;
+      tmp_stgmethod = 0;  // indicates uncompressed data
+
+    } // end if
+    else
+    { // OK, we want to compress the data
+      logger.debug("stored as COMPRESSED data");
+
       try
       { // create a compressor into a memory buffer
 	ByteArrayOutputStream bytestm = new ByteArrayOutputStream(length);
@@ -887,6 +904,9 @@ class TopicMessageUserContextImpl implements TopicMessageContext
 	real_length = buffer.length;
 	real_data = new ByteArrayInputStream(buffer);
 	gzipstm.close();
+	if (logger.isDebugEnabled())
+	  logger.debug("org size " + length + ", compr size = " + real_length + " ("
+		       + String.valueOf(100 - ((real_length * 100) / length)) + "% reduction)");
 
       } // end try
       catch (IOException ioe)
@@ -896,6 +916,10 @@ class TopicMessageUserContextImpl implements TopicMessageContext
 
       } // end catch
 
+      tmp_stgmethod = 1;  // indicates compressed data
+
+    } // end else
+    
     Connection conn = null;
     AuditRecord ar = null;
 
@@ -917,9 +941,11 @@ class TopicMessageUserContextImpl implements TopicMessageContext
       StringBuffer sql = new StringBuffer("INSERT INTO postattach (postid, datalen, last_hit, stgmethod, "
 					  + "filename, mimetype, data) VALUES (");
       sql.append(postid).append(", ").append(length).append(", '");
-      sql.append(SQLUtil.encodeDate(new java.util.Date())).append("', 1, '");
+      sql.append(SQLUtil.encodeDate(new java.util.Date())).append("', ").append(tmp_stgmethod).append(", '");
       sql.append(SQLUtil.encodeString(file)).append("', '").append(SQLUtil.encodeString(m_type));
       sql.append("', ?);");
+      if (logger.isDebugEnabled())
+	logger.debug("SQL: " + sql.toString());
 
       // Prepare the statement, set the BLOB parameter, and execute it.
       PreparedStatement stmt = conn.prepareStatement(sql.toString());
@@ -927,15 +953,16 @@ class TopicMessageUserContextImpl implements TopicMessageContext
       stmt.executeUpdate();
 
       // Save off the local attachment values.
+      logger.debug("... attachment completed successfully");
       datalen = length;
       filename = file;
       mimetype = m_type;
-      stgmethod = 1;
+      stgmethod = tmp_stgmethod;
 
       // Generate an audit record indicating what we did.
       ar = new AuditRecord(AuditRecord.UPLOAD_ATTACHMENT,conf.realUID(),conf.userRemoteAddress(),
 			   conf.realSIGID(),"conf=" + conf.realConfID() + ",post=" + postid,
-			   "len=" + length + ",type=" + m_type + ",name=" + file);
+			   "len=" + length + ",type=" + m_type + ",name=" + file + ",method=" + tmp_stgmethod);
 
     } // end try
     catch (SQLException e)

src/com/silverwrist/venice/core/impl/VeniceEngineImpl.java

@@ -382,7 +382,7 @@ public class VeniceEngineImpl implements VeniceEngine, EngineBackend
    *--------------------------------------------------------------------------------
    */
 
-  private static Category logger = Category.getInstance(VeniceEngineImpl.class.getName());
+  private static Category logger = Category.getInstance(VeniceEngineImpl.class);
 
   private static final String AUTH_ALPHABET =
       "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz./";
@@ -411,6 +411,7 @@ public class VeniceEngineImpl implements VeniceEngine, EngineBackend
   private Hashtable sidebox_ids = new Hashtable();    // maps sidebox IDs to MasterSideBox objects
   private LinkedList cache_fp_posts = new LinkedList();  // all posts that have been published to front page
   private boolean cache_fp_posts_busy = false;        // busy flag for above vector
+  private HashSet no_compress_types = new HashSet();  // the file types that can't be compressed
 
   /*--------------------------------------------------------------------------------
    * Constructor
@@ -560,6 +561,46 @@ public class VeniceEngineImpl implements VeniceEngine, EngineBackend
 
       } // end for
 
+      Element upload_sect = root_h.getSubElement("upload");
+      if (upload_sect==null)
+      { // unable to find the "upload" section
+	logger.fatal("config document has no <upload/> section");
+	throw new ConfigException("no <upload/> section found in config file",root);
+
+      } // end if
+
+      // Look for a "no-compress" blacklist.
+      DOMElementHelper upload_sect_h = new DOMElementHelper(upload_sect);
+      Element no_compress_sect = upload_sect_h.getSubElement("no-compress");
+      if (no_compress_sect!=null)
+      { // Initialize the no-compress list.
+	NodeList nc_nodes = no_compress_sect.getChildNodes();
+	for (i=0; i<nc_nodes.getLength(); i++)
+	{ // retrieve all entries from the list
+	  Node ncn = nc_nodes.item(i);
+	  if (ncn.getNodeType()==Node.ELEMENT_NODE)
+	  { // get the element
+	    DOMElementHelper ncel = new DOMElementHelper((Element)ncn);
+	    if (ncel.getElement().getNodeName().equals("type"))
+	    { // add a new element to the list
+	      no_compress_types.add(ncel.getElementText().trim().toLowerCase());
+
+	    } // end if
+	    else
+	    { // this is bad - bail out!
+	      logger.fatal("spurious element in <no-compress/> section");
+	      throw new ConfigException("<no-compress/> section contains spurious element",no_compress_sect);
+
+	    } // end else
+
+	  } // end if
+	  // else skip it
+
+	} // end for
+
+      } // end if
+      // else no list, just proceed
+      
       Element msg_sect = root_h.getSubElement("messages");
       if (msg_sect==null)
       { // unable to find the database section
@@ -1960,4 +2001,21 @@ public class VeniceEngineImpl implements VeniceEngine, EngineBackend
 
   } // end isValidRandomAuthString
 
+  public boolean isNoCompressMimeType(String type)
+  {
+    if (no_compress_types.isEmpty())
+      return false;  // can't possibly be no-compress
+
+    // strip the MIME type name down to basics
+    int psem = type.indexOf(';');
+    String real_type;
+    if (psem>=0)
+      real_type = type.substring(0,psem).trim().toLowerCase();
+    else
+      real_type = type.trim().toLowerCase();
+
+    return no_compress_types.contains(real_type);
+
+  } // end isNoCompressMimeType
+
 } // end class VeniceEngineImpl

src/com/silverwrist/venice/servlets/Attachment.java

@@ -34,7 +34,7 @@ public class Attachment extends VeniceServlet
    *--------------------------------------------------------------------------------
    */
 
-  private static Category logger = Category.getInstance(Attachment.class.getName());
+  private static Category logger = Category.getInstance(Attachment.class);
 
   /*--------------------------------------------------------------------------------
    * Overrides from class HttpServlet
@@ -102,25 +102,44 @@ public class Attachment extends VeniceServlet
 				       VeniceEngine engine, UserContext user, RenderData rdat)
     throws ServletException, IOException, VeniceServletResult
   {
+    logger.debug("Attachment.doVenicePost: entry");
+
     // Get target URL for the operation
     if (mphandler.isFileParam("target"))
+    { // bogus target URL
+      logger.error("Internal Error: 'target' should be a normal param");
       return new ErrorBox(null,"Internal Error: 'target' should be a normal param","top");
+
+    } // end if
+
     String target = mphandler.getValue("target");
+    if (logger.isDebugEnabled())
+      logger.debug("Target URL: " + target);
     setMyLocation(request,target);
 
     // also check on file parameter status
     if (!(mphandler.isFileParam("thefile")))
+    { // bogus file parameter
+      logger.error("Internal Error: 'thefile' should be a file param");
       return new ErrorBox(null,"Internal Error: 'thefile' should be a file param",target);
 
+    } // end if
+
     // get the SIG
     SIGContext sig = getSIGParameter(mphandler,user,true,"top");
+    if (logger.isDebugEnabled())
+      logger.debug("SIG param: #" + sig.getSIGID());
     changeMenuSIG(request,sig);
 
     // get the conference
     ConferenceContext conf = getConferenceParameter(mphandler,sig,true,"top");
+    if (logger.isDebugEnabled())
+      logger.debug("Conference param: #" + conf.getConfID());
 
     // get the message we want to use
     TopicMessageContext msg = getMessageParameter(mphandler,conf,true,"top");
+    if (logger.isDebugEnabled())
+      logger.debug("Message param: #" + msg.getPostID());
 
     try
     { // attach the data to the message!
@@ -130,20 +149,24 @@ public class Attachment extends VeniceServlet
     } // end try
     catch (ServletMultipartException smpe)
     { // error processing the file parameter data
+      logger.error("Servlet multipart error:",smpe);
       return new ErrorBox(null,"Internal Error: " + smpe.getMessage(),target);
 
     } // end catch
     catch (AccessError ae)
     { // access error posting the attachment data
+      logger.error("Access error:",ae);
       return new ErrorBox("Access Error",ae.getMessage(),target);
 
     } // end catch
     catch (DataException de)
     { // error storing the attachment in the database
+      logger.error("DataException:",de);
       return new ErrorBox("Database Error","Database error storing attachment: " + de.getMessage(),target);
 
     } // end catch
 
+    logger.debug("Attachment.doVenicePost: done!");
     throw new RedirectResult(target);  // get back, get back, get back to where you once belonged
 
   } // end doVenicePost