You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by ol...@apache.org on 2008/12/07 16:26:00 UTC

svn commit: r724142 - in /james/mime4j/trunk/src: main/java/org/apache/james/mime4j/field/ main/java/org/apache/james/mime4j/message/ main/java/org/apache/james/mime4j/parser/ test/java/org/apache/james/mime4j/field/

Author: olegk
Date: Sun Dec  7 07:26:00 2008
New Revision: 724142

URL: http://svn.apache.org/viewvc?rev=724142&view=rev
Log:
MIME4J-90: Consistent parsing of header field names
Contributed by Markus Wiederkehr <markus.wiederkehr at gmail.com>

Modified:
    james/mime4j/trunk/src/main/java/org/apache/james/mime4j/field/Field.java
    james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/Header.java
    james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/SimpleContentHandler.java
    james/mime4j/trunk/src/main/java/org/apache/james/mime4j/parser/AbstractEntity.java
    james/mime4j/trunk/src/test/java/org/apache/james/mime4j/field/FieldTest.java

Modified: james/mime4j/trunk/src/main/java/org/apache/james/mime4j/field/Field.java
URL: http://svn.apache.org/viewvc/james/mime4j/trunk/src/main/java/org/apache/james/mime4j/field/Field.java?rev=724142&r1=724141&r2=724142&view=diff
==============================================================================
--- james/mime4j/trunk/src/main/java/org/apache/james/mime4j/field/Field.java (original)
+++ james/mime4j/trunk/src/main/java/org/apache/james/mime4j/field/Field.java Sun Dec  7 07:26:00 2008
@@ -22,8 +22,6 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import org.apache.james.mime4j.MimeException;
-
 /**
  * The base class of all field classes.
  *
@@ -52,7 +50,7 @@
                                         "Content-Transfer-Encoding";
     
     private static final String FIELD_NAME_PATTERN = 
-        "^([\\x21-\\x39\\x3b-\\x7e]+)[ \t]*:";
+        "^([\\x21-\\x39\\x3b-\\x7e]+):";
     private static final Pattern fieldNamePattern = 
         Pattern.compile(FIELD_NAME_PATTERN);
         
@@ -82,9 +80,9 @@
      * 
      * @param raw the string to parse.
      * @return a <code>Field</code> instance.
-     * @throws MimeException on parse errors.
+     * @throws IllegalArgumentException on parse errors.
      */
-    public static Field parse(final String raw) throws MimeException {
+    public static Field parse(final String raw) {
         
         /*
          * Unfold the field.
@@ -96,7 +94,11 @@
          */
         final Matcher fieldMatcher = fieldNamePattern.matcher(unfolded);
         if (!fieldMatcher.find()) {
-            throw new MimeException("Invalid field in string");
+            // We don't have to throw a MimeException because this error can
+            // never happen when Field.parse() is called by a ContentHandler.
+            // org.apache.james.mime4j.parser.AbstractEntity drops header
+            // lines that do not start with a valid field name.
+            throw new IllegalArgumentException("Invalid field in string");
         }
         final String name = fieldMatcher.group(1);
         

Modified: james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/Header.java
URL: http://svn.apache.org/viewvc/james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/Header.java?rev=724142&r1=724141&r2=724142&view=diff
==============================================================================
--- james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/Header.java (original)
+++ james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/Header.java Sun Dec  7 07:26:00 2008
@@ -76,7 +76,7 @@
                 parser.stop();
             }
             @Override
-            public void field(String fieldData) throws MimeException {
+            public void field(String fieldData) {
                 addField(Field.parse(fieldData));
             }
         });

Modified: james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/SimpleContentHandler.java
URL: http://svn.apache.org/viewvc/james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/SimpleContentHandler.java?rev=724142&r1=724141&r2=724142&view=diff
==============================================================================
--- james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/SimpleContentHandler.java (original)
+++ james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/SimpleContentHandler.java Sun Dec  7 07:26:00 2008
@@ -19,7 +19,6 @@
 
 package org.apache.james.mime4j.message;
 
-import org.apache.james.mime4j.MimeException;
 import org.apache.james.mime4j.decoder.Base64InputStream;
 import org.apache.james.mime4j.decoder.QuotedPrintableInputStream;
 import org.apache.james.mime4j.descriptor.BodyDescriptor;
@@ -75,7 +74,7 @@
      * @see org.apache.james.mime4j.parser.AbstractContentHandler#field(java.lang.String)
      */
     @Override
-    public final void field(String fieldData) throws MimeException {
+    public final void field(String fieldData) {
         currHeader.addField(Field.parse(fieldData));
     }
 

Modified: james/mime4j/trunk/src/main/java/org/apache/james/mime4j/parser/AbstractEntity.java
URL: http://svn.apache.org/viewvc/james/mime4j/trunk/src/main/java/org/apache/james/mime4j/parser/AbstractEntity.java?rev=724142&r1=724141&r2=724142&view=diff
==============================================================================
--- james/mime4j/trunk/src/main/java/org/apache/james/mime4j/parser/AbstractEntity.java (original)
+++ james/mime4j/trunk/src/main/java/org/apache/james/mime4j/parser/AbstractEntity.java Sun Dec  7 07:26:00 2008
@@ -193,7 +193,7 @@
             boolean valid = true;
             field = fieldbuf.toString();
             int pos = fieldbuf.indexOf(':');
-            if (pos == -1) {
+            if (pos <= 0) {
                 monitor(Event.INALID_HEADER);
                 valid = false;
             } else {

Modified: james/mime4j/trunk/src/test/java/org/apache/james/mime4j/field/FieldTest.java
URL: http://svn.apache.org/viewvc/james/mime4j/trunk/src/test/java/org/apache/james/mime4j/field/FieldTest.java?rev=724142&r1=724141&r2=724142&view=diff
==============================================================================
--- james/mime4j/trunk/src/test/java/org/apache/james/mime4j/field/FieldTest.java (original)
+++ james/mime4j/trunk/src/test/java/org/apache/james/mime4j/field/FieldTest.java Sun Dec  7 07:26:00 2008
@@ -19,7 +19,6 @@
 
 package org.apache.james.mime4j.field;
 
-import org.apache.james.mime4j.MimeException;
 import org.apache.james.mime4j.field.ContentTransferEncodingField;
 import org.apache.james.mime4j.field.ContentTypeField;
 import org.apache.james.mime4j.field.Field;
@@ -46,9 +45,9 @@
         
         try {
             f = Field.parse("Yada yada yada");
-            fail("MimeException not thrown when using an invalid "
+            fail("IllegalArgumentException not thrown when using an invalid "
                     + "field");
-        } catch (MimeException e) {
+        } catch (IllegalArgumentException e) {
         }
     }
 



---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Re: svn commit: r724142

Posted by Robert Burrell Donkin <ro...@gmail.com>.
On Sun, Dec 7, 2008 at 4:08 PM, Oleg Kalnichevski <ol...@apache.org> wrote:
> Robert Burrell Donkin wrote:
>>
>> i'm unhappy about the impact of this resolution for direct users of this
>> method
>>
>
> Robert,
>
> What would you like done differently?

it's quite a change for a public API given that mail protocol users
may be relying on catching that MimeException. i dislike throwing
runtimes because this usually means that the processing is dead and
the mail will be impossible to process. unreadable fields can usually
be ignored when processing so recovery is possible.

i'd prefer to see a name change for the field to go with the change in
behaviour, that way it's obvious that it's changed. i haven't followed
all the discussions too deeply so there may some other strategies
which may works equally well. maintaining the original method might
also be useful if it's used elsewhere in James (i'll need to check
that).

(it also needs to be documented etc but i'm happy to dive in and fix that)

- robert

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Re: svn commit: r724142

Posted by Oleg Kalnichevski <ol...@apache.org>.
Robert Burrell Donkin wrote:
> i'm unhappy about the impact of this resolution for direct users of this method
> 

Robert,

What would you like done differently?

Oleg


> - robert
> 
> On Sun, Dec 7, 2008 at 3:26 PM,  <ol...@apache.org> wrote:
>> Author: olegk
>> Date: Sun Dec  7 07:26:00 2008
>> New Revision: 724142
>>
>> URL: http://svn.apache.org/viewvc?rev=724142&view=rev
>> Log:
>> MIME4J-90: Consistent parsing of header field names
>> Contributed by Markus Wiederkehr <markus.wiederkehr at gmail.com>
>>
>> Modified:
>>    james/mime4j/trunk/src/main/java/org/apache/james/mime4j/field/Field.java
>>    james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/Header.java
>>    james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/SimpleContentHandler.java
>>    james/mime4j/trunk/src/main/java/org/apache/james/mime4j/parser/AbstractEntity.java
>>    james/mime4j/trunk/src/test/java/org/apache/james/mime4j/field/FieldTest.java
>>

...

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Re: svn commit: r724142 - in /james/mime4j/trunk/src: main/java/org/apache/james/mime4j/field/ main/java/org/apache/james/mime4j/message/ main/java/org/apache/james/mime4j/parser/ test/java/org/apache/james/mime4j/field/

Posted by Robert Burrell Donkin <ro...@gmail.com>.
i'm unhappy about the impact of this resolution for direct users of this method

- robert

On Sun, Dec 7, 2008 at 3:26 PM,  <ol...@apache.org> wrote:
> Author: olegk
> Date: Sun Dec  7 07:26:00 2008
> New Revision: 724142
>
> URL: http://svn.apache.org/viewvc?rev=724142&view=rev
> Log:
> MIME4J-90: Consistent parsing of header field names
> Contributed by Markus Wiederkehr <markus.wiederkehr at gmail.com>
>
> Modified:
>    james/mime4j/trunk/src/main/java/org/apache/james/mime4j/field/Field.java
>    james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/Header.java
>    james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/SimpleContentHandler.java
>    james/mime4j/trunk/src/main/java/org/apache/james/mime4j/parser/AbstractEntity.java
>    james/mime4j/trunk/src/test/java/org/apache/james/mime4j/field/FieldTest.java
>
> Modified: james/mime4j/trunk/src/main/java/org/apache/james/mime4j/field/Field.java
> URL: http://svn.apache.org/viewvc/james/mime4j/trunk/src/main/java/org/apache/james/mime4j/field/Field.java?rev=724142&r1=724141&r2=724142&view=diff
> ==============================================================================
> --- james/mime4j/trunk/src/main/java/org/apache/james/mime4j/field/Field.java (original)
> +++ james/mime4j/trunk/src/main/java/org/apache/james/mime4j/field/Field.java Sun Dec  7 07:26:00 2008
> @@ -22,8 +22,6 @@
>  import java.util.regex.Matcher;
>  import java.util.regex.Pattern;
>
> -import org.apache.james.mime4j.MimeException;
> -
>  /**
>  * The base class of all field classes.
>  *
> @@ -52,7 +50,7 @@
>                                         "Content-Transfer-Encoding";
>
>     private static final String FIELD_NAME_PATTERN =
> -        "^([\\x21-\\x39\\x3b-\\x7e]+)[ \t]*:";
> +        "^([\\x21-\\x39\\x3b-\\x7e]+):";
>     private static final Pattern fieldNamePattern =
>         Pattern.compile(FIELD_NAME_PATTERN);
>
> @@ -82,9 +80,9 @@
>      *
>      * @param raw the string to parse.
>      * @return a <code>Field</code> instance.
> -     * @throws MimeException on parse errors.
> +     * @throws IllegalArgumentException on parse errors.
>      */
> -    public static Field parse(final String raw) throws MimeException {
> +    public static Field parse(final String raw) {
>
>         /*
>          * Unfold the field.
> @@ -96,7 +94,11 @@
>          */
>         final Matcher fieldMatcher = fieldNamePattern.matcher(unfolded);
>         if (!fieldMatcher.find()) {
> -            throw new MimeException("Invalid field in string");
> +            // We don't have to throw a MimeException because this error can
> +            // never happen when Field.parse() is called by a ContentHandler.
> +            // org.apache.james.mime4j.parser.AbstractEntity drops header
> +            // lines that do not start with a valid field name.
> +            throw new IllegalArgumentException("Invalid field in string");
>         }
>         final String name = fieldMatcher.group(1);
>
>
> Modified: james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/Header.java
> URL: http://svn.apache.org/viewvc/james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/Header.java?rev=724142&r1=724141&r2=724142&view=diff
> ==============================================================================
> --- james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/Header.java (original)
> +++ james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/Header.java Sun Dec  7 07:26:00 2008
> @@ -76,7 +76,7 @@
>                 parser.stop();
>             }
>             @Override
> -            public void field(String fieldData) throws MimeException {
> +            public void field(String fieldData) {
>                 addField(Field.parse(fieldData));
>             }
>         });
>
> Modified: james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/SimpleContentHandler.java
> URL: http://svn.apache.org/viewvc/james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/SimpleContentHandler.java?rev=724142&r1=724141&r2=724142&view=diff
> ==============================================================================
> --- james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/SimpleContentHandler.java (original)
> +++ james/mime4j/trunk/src/main/java/org/apache/james/mime4j/message/SimpleContentHandler.java Sun Dec  7 07:26:00 2008
> @@ -19,7 +19,6 @@
>
>  package org.apache.james.mime4j.message;
>
> -import org.apache.james.mime4j.MimeException;
>  import org.apache.james.mime4j.decoder.Base64InputStream;
>  import org.apache.james.mime4j.decoder.QuotedPrintableInputStream;
>  import org.apache.james.mime4j.descriptor.BodyDescriptor;
> @@ -75,7 +74,7 @@
>      * @see org.apache.james.mime4j.parser.AbstractContentHandler#field(java.lang.String)
>      */
>     @Override
> -    public final void field(String fieldData) throws MimeException {
> +    public final void field(String fieldData) {
>         currHeader.addField(Field.parse(fieldData));
>     }
>
>
> Modified: james/mime4j/trunk/src/main/java/org/apache/james/mime4j/parser/AbstractEntity.java
> URL: http://svn.apache.org/viewvc/james/mime4j/trunk/src/main/java/org/apache/james/mime4j/parser/AbstractEntity.java?rev=724142&r1=724141&r2=724142&view=diff
> ==============================================================================
> --- james/mime4j/trunk/src/main/java/org/apache/james/mime4j/parser/AbstractEntity.java (original)
> +++ james/mime4j/trunk/src/main/java/org/apache/james/mime4j/parser/AbstractEntity.java Sun Dec  7 07:26:00 2008
> @@ -193,7 +193,7 @@
>             boolean valid = true;
>             field = fieldbuf.toString();
>             int pos = fieldbuf.indexOf(':');
> -            if (pos == -1) {
> +            if (pos <= 0) {
>                 monitor(Event.INALID_HEADER);
>                 valid = false;
>             } else {
>
> Modified: james/mime4j/trunk/src/test/java/org/apache/james/mime4j/field/FieldTest.java
> URL: http://svn.apache.org/viewvc/james/mime4j/trunk/src/test/java/org/apache/james/mime4j/field/FieldTest.java?rev=724142&r1=724141&r2=724142&view=diff
> ==============================================================================
> --- james/mime4j/trunk/src/test/java/org/apache/james/mime4j/field/FieldTest.java (original)
> +++ james/mime4j/trunk/src/test/java/org/apache/james/mime4j/field/FieldTest.java Sun Dec  7 07:26:00 2008
> @@ -19,7 +19,6 @@
>
>  package org.apache.james.mime4j.field;
>
> -import org.apache.james.mime4j.MimeException;
>  import org.apache.james.mime4j.field.ContentTransferEncodingField;
>  import org.apache.james.mime4j.field.ContentTypeField;
>  import org.apache.james.mime4j.field.Field;
> @@ -46,9 +45,9 @@
>
>         try {
>             f = Field.parse("Yada yada yada");
> -            fail("MimeException not thrown when using an invalid "
> +            fail("IllegalArgumentException not thrown when using an invalid "
>                     + "field");
> -        } catch (MimeException e) {
> +        } catch (IllegalArgumentException e) {
>         }
>     }
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org