commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Daniel F. Savarese" <...@savarese.org>
Subject Commons Net issues
Date Tue, 12 Apr 2011 17:59:05 GMT

1. NET-396 POP3.setState() should not be public

   I don't think I can revert a change without first getting permission
   from the committer who made it.  I believe this change needs to be
   reverted for the reasons stated in my issue comment.  sebb, if you
   object, let's discuss it.  If you don't, please let me know so I can
   revert the change.  I understand why you made the change.  However,
   even though base protocol classes are intended primarily for use by
   the libary, they are not exclusively for use by the library.  I think
   changes of this sort that may make existing user code non-functional
   should be left for a redesign and rewrite of Commons Net.  They don't
   fix anything and may only break things.  In the case of this change,
   I know it will break things for one of my former customers.

2. NET-393 Should the sendCommandWithID() methods be public?

   I'm not asking to revert this change because I'm not familiar enough
   with the IMAP code.  However, the person who contributed the IMAP code made
   the IMAP base protocol class consistent with the pattern followed by the
   existing base protocol classes, whereby all protocol manipulation functions
   are supposed to be public.  As with the POP3 case, methods that have been
   made public to allow developers with special needs to implement their own
   protocol client code via aggregation should not be made non-public to
   satisfy a sense of aesthetics or to protect programmers from doing stupid
   things.  Unlike the POP3 case, I don't know of anyone for whom this change
   will cause a problem.

3. NET-402 BufferedReader used for control channel, which does not follow
           the standard

   The premise of this issue, at least with respect to NNTP and SMTP, is
   misleading.  Both NNTP and SMTP prohibit the occurrence of a bare CR
   or LF.  Therefore, the only standards-compliant behavior is to reject any
   occurrences of bare CR or LF characters (which is undesirable if software
   is to function in the presence of errors).  Recognizing only CRLF
   as a valid line terminator does not make the code standards compliant
   because now bare CR's and LF's are being interpreted as either
   command contents or data, both of which are prohibited.

   Either you assume the server is standards compliant or it is not.  If
   you assume it is standards compliant, then using BufferedReader is not
   a problem.  If you assume it is not standards compliant, then 
   BufferedReader and the added CRLFLineReader both have problems in different
   situations.  Yada, yada, yada.  All of this has been considered before
   and the decision was made to use BufferedReader because various
   servers for a number of protocols at the time were known to be sending
   bare LF's as line terminators instead of CRLF's.  If it is more prevalent
   for non-conforming servers to include bare CR or LF characters as
   data instead of the end of a command, then it's better to accept them
   as data.  But that wasn't the argument made for making the change and
   would seem to apply to NNTP and POP3, but not necessarily SMTP.  A
   complete solution (that I don't feel is worth implementing) would be to
   identify what type of non-conformance one is faced with when encountering
   a bare CR or LF (are we reading a command or data, does it look like we're
   at the end of a command, etc. and decide what to do with the bare CR or LF).

   It's a waste of time to revert this change as it only replaces one
   problem with another.  However, if we are to build any sense of community
   amongst people who have contributed to Commons Net development and
   encourage more participation, this is exactly the sort of issue that
   should be discussed on the dev list first before taking action.
   Ultimately, the same decision and result may be reached, but at least it
   will have been decided via a consensus-based approach.

I hardly comment about Commons Net changes because I don't feel
I contribute enough anymore to have a say.  But #1 is important because
the POP3 class can be and has been used outside of the libarary.  I tend
to get concerned only about changes that may adversely impact existing
users and try to refrain from kibitzing needlessly when other people are
doing all the hard work.  But Commons Net has become more and more
fragile as it's been touched by so many different hands under different
design assumptions.  In the interest of not breaking functionality for
the users, I recommend we not make changes like 1 and 2 unless there's
a known behavioral bug (which should be impossible for that class of change).
I didn't really want to broach 3 because the issue itself impacts only
exchanges with non-conforming servers and I have no desire to make
mountains out of molehills.  But I felt I should say something because
not discussing issues like this before making a change has whittled away
the committership, leaving us with one active committer (sebb) and one
largely inactive backup committer (me).  This isn't a phenomenon specific
to Commons Net and it's not any easy one to solve because everyone has
different notions of what sort of changes require discussion.

daniel


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


Mime
View raw message