You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@synapse.apache.org by Andreas Veithen <an...@skynet.be> on 2008/03/19 21:15:00 UTC
Re: svn commit: r638978 - /synapse/trunk/java/modules/transports/src/main/java/org/apache/synapse/transport/mail/MailUtils.java
Ruwan,
I think there are some other things that should be cleaned up in this
piece of code:
* The condition mbp != null is useless: if mbp is null, the expression
mbp.getContentType() will cause a NPE before reaching the if
statement. Anyway, the result of MimeBodyPart#getBodyPart is never null.
* plain/text is not a valid content type, so this condition is
confusing and should be removed.
* The first if statement contains contType != null. If contType is
null, the expression contType.indexOf(...) in the second if statement
will always throw a NPE. Anyway, the result of getContentType is never
null.
* After the for loop, the variable firstTextPart will actually store a
reference to the *last* text part.
Andreas
On 19 Mar 2008, at 20:33, ruwan@apache.org wrote:
> Author: ruwan
> Date: Wed Mar 19 12:33:50 2008
> New Revision: 638978
>
> URL: http://svn.apache.org/viewvc?rev=638978&view=rev
> Log:
> Fixing issue SYNAPSE-256, (it seems gmail send the contentType as
> text/plain rather than plain/text)
>
> Modified:
> synapse/trunk/java/modules/transports/src/main/java/org/apache/
> synapse/transport/mail/MailUtils.java
>
> Modified: synapse/trunk/java/modules/transports/src/main/java/org/
> apache/synapse/transport/mail/MailUtils.java
> URL: http://svn.apache.org/viewvc/synapse/trunk/java/modules/transports/src/main/java/org/apache/synapse/transport/mail/MailUtils.java?rev=638978&r1=638977&r2=638978&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- synapse/trunk/java/modules/transports/src/main/java/org/apache/
> synapse/transport/mail/MailUtils.java (original)
> +++ synapse/trunk/java/modules/transports/src/main/java/org/apache/
> synapse/transport/mail/MailUtils.java Wed Mar 19 12:33:50 2008
> @@ -79,12 +79,15 @@
>
> contType.indexOf(SOAP12Constants.SOAP_12_CONTENT_TYPE) != -1)) {
> // this part is a SOAP 11 or 12 payload,
> treat this as the message
> return mbp.getInputStream();
> - } else if (mbp == null &&
> contType.indexOf("plain/text") != -1) {
> + } else if (mbp != null &&
> (contType.indexOf("plain/text") != -1
> + || contType.indexOf("text/plain") !
> = -1)) {
> firstTextPart = mbp;
> }
> }
> // if a soap 11 or soap12 payload was not found,
> treat first text part as message
> - return firstTextPart.getInputStream();
> + if (firstTextPart != null) {
> + return firstTextPart.getInputStream();
> + }
>
> } else {
> return ((Message) message).getInputStream();
> @@ -94,6 +97,7 @@
> handleException("Error creating an input stream to : " +
> ((Message) message).getMessageNumber(), e);
> }
> +
> return null;
> }
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@synapse.apache.org
For additional commands, e-mail: dev-help@synapse.apache.org
Re: svn commit: r638978 -
/synapse/trunk/java/modules/transports/src/main/java/org/apache/synapse/transport/mail/MailUtils.java
Posted by Andreas Veithen <an...@skynet.be>.
Quoting Ruwan Linton <ru...@gmail.com>:
>> * plain/text is not a valid content type, so this condition is
>> confusing and should be removed.
>
>
> I thought it is a valid contentType because it is the one which is already
> there in the code but found that gmail sends text/plain when I debugged the
> issue, so to do a lesser harm :) I added the text/plain without removing the
> existing plain/text. I wonder how this was working then???
Well, I think it never worked. The expression initially was like that:
mbp == null && contType.indexOf("plain/text") != -1
Since mbp cannot be null, the code was never actually executed.
Andreas
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@synapse.apache.org
For additional commands, e-mail: dev-help@synapse.apache.org
Re: svn commit: r638978 - /synapse/trunk/java/modules/transports/src/main/java/org/apache/synapse/transport/mail/MailUtils.java
Posted by Ruwan Linton <ru...@gmail.com>.
Hi Andreas,
Thanks for the comments....
On Thu, Mar 20, 2008 at 1:45 AM, Andreas Veithen <an...@skynet.be>
wrote:
> Ruwan,
>
> I think there are some other things that should be cleaned up in this
> piece of code:
>
> * The condition mbp != null is useless: if mbp is null, the expression
> mbp.getContentType() will cause a NPE before reaching the if
> statement. Anyway, the result of MimeBodyPart#getBodyPart is never null.
Yeah, you are correct.
>
> * plain/text is not a valid content type, so this condition is
> confusing and should be removed.
I thought it is a valid contentType because it is the one which is already
there in the code but found that gmail sends text/plain when I debugged the
issue, so to do a lesser harm :) I added the text/plain without removing the
existing plain/text. I wonder how this was working then???
>
> * The first if statement contains contType != null. If contType is
> null, the expression contType.indexOf(...) in the second if statement
> will always throw a NPE. Anyway, the result of getContentType is never
> null.
OK...
>
> * After the for loop, the variable firstTextPart will actually store a
> reference to the *last* text part.
Shall we change it to first text part, or do we need to merge all the text
parts, Asankha WDYT?
Thanks,
Ruwan
>
>
> Andreas
>
> On 19 Mar 2008, at 20:33, ruwan@apache.org wrote:
> > Author: ruwan
> > Date: Wed Mar 19 12:33:50 2008
> > New Revision: 638978
> >
> > URL: http://svn.apache.org/viewvc?rev=638978&view=rev
> > Log:
> > Fixing issue SYNAPSE-256, (it seems gmail send the contentType as
> > text/plain rather than plain/text)
> >
> > Modified:
> > synapse/trunk/java/modules/transports/src/main/java/org/apache/
> > synapse/transport/mail/MailUtils.java
> >
> > Modified: synapse/trunk/java/modules/transports/src/main/java/org/
> > apache/synapse/transport/mail/MailUtils.java
> > URL:
> http://svn.apache.org/viewvc/synapse/trunk/java/modules/transports/src/main/java/org/apache/synapse/transport/mail/MailUtils.java?rev=638978&r1=638977&r2=638978&view=diff
> > =
> > =
> > =
> > =
> > =
> > =
> > =
> > =
> > ======================================================================
> > --- synapse/trunk/java/modules/transports/src/main/java/org/apache/
> > synapse/transport/mail/MailUtils.java (original)
> > +++ synapse/trunk/java/modules/transports/src/main/java/org/apache/
> > synapse/transport/mail/MailUtils.java Wed Mar 19 12:33:50 2008
> > @@ -79,12 +79,15 @@
> >
> > contType.indexOf(SOAP12Constants.SOAP_12_CONTENT_TYPE) != -1)) {
> > // this part is a SOAP 11 or 12 payload,
> > treat this as the message
> > return mbp.getInputStream();
> > - } else if (mbp == null &&
> > contType.indexOf("plain/text") != -1) {
> > + } else if (mbp != null &&
> > (contType.indexOf("plain/text") != -1
> > + || contType.indexOf("text/plain") !
> > = -1)) {
> > firstTextPart = mbp;
> > }
> > }
> > // if a soap 11 or soap12 payload was not found,
> > treat first text part as message
> > - return firstTextPart.getInputStream();
> > + if (firstTextPart != null) {
> > + return firstTextPart.getInputStream();
> > + }
> >
> > } else {
> > return ((Message) message).getInputStream();
> > @@ -94,6 +97,7 @@
> > handleException("Error creating an input stream to : " +
> > ((Message) message).getMessageNumber(), e);
> > }
> > +
> > return null;
> > }
> >
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@synapse.apache.org
> For additional commands, e-mail: dev-help@synapse.apache.org
>
>
--
Ruwan Linton
http://www.wso2.org - "Oxygenating the Web Services Platform"