Harmeet,
> bugs: > Good stuff about the bugs. Group command was a miss and LIST seems to have > been a really bad one. It was half correct half wrong, but the net effect > was all wrong. The strange thing is that I have tested with Outlook, > Netsacpe and KClient and still it did not show up. Others tested with news > group clients and it did not show either. IMHO, there is very little use in testing protocol servers with generic clients without doing lower level protocol testing first. Clients are always designed to be robust in the face of misbehaving servers. You start by getting the protocol exactly right (usually by automated tests or by simple telnet sessions) and then you test with common clients to find any gotchas - either errors in your implementation or client quirks. I'm not surprised that the clients missed these issues. > authservice: > You are right about the AuthService. Should never have been a service, > however I think it is a useful abstraction. Would it be possible to have > an > AuthServiceFactory as a service instead of AuthService. UsersStore etc. > are > ideally not a concern of the Handler, but a concern of AuthService. It > should be easy fix AuthService this way and still have the advantages of > the > abstraction. IMHO the AuthService is not worth fixing, for all the reasons I laid out in my previous email. I have very strong feelings on this matter - I have an extensive background in authentication and authorization - and really feel that the AuthService should be retired. Designing and implementing a pluggable authentication/authorization framework is not trivial. Moreover there are a number of generally accepted tools (JAAS) and others under development (JSR 115) that could be relevant. This needs to be designed properly, not patched together at the end of a release cycle to fix something that didn't work. And considering that this bug has been languishing for nine months or so, I don't think there has been any extensive exercise of this code. It can wait until next version. > comments: > prefer the older style more. We can define our own coding standard if you > prefer that. Copying a Jakarta coding style if that involves cut paste > comments from base classes seems less than good. @see is the lesser of two > evils if there is no agreement. Let us reduce comments, unless they are > really helpful. A comment like '// see section <...>' is far more helpful > to > me than say javadoc style comments on a private variable. Too much chaff. > I prefer 'C: ' and 'S: ' to indicate the client and server side, it seems > more standard and is less verbose. Just as Danny feels strongly about some elements of the Jakarta project, this is one I feel we must hold to. Javadoc is one of the real strengths of the Java language, and I want to take full advantage of it. As I mentioned before, I have no objection to the use of @see tags to reduce the size of the comments - must they must be present. I also believe that focusing on the comments for overridden/inherited methods misses the big picture - that's not even close to the majority of methods commented in this or in other documentation changes. I've already expressed the reasons why I believe that extensive commenting is essential to a healthy open source project, so I won't repeat them here. Suffice to say, I'm very happy with the improvement in source code comments over the last few months and I hope to see further improvement before release. > malformed messages: > right error messages are being returned in most handlers. If they are not > then that should be fixed. Your patch does not seem to have changed > malformed headers significantly. Did you have another patch that address > this. As I said, this is an open issue. But basically, once you get past the parseCommand method and into a doXXX method, there appears to be no real syntax checking. There were random RuntimeExceptions thrown (see the former check method) as well as potential NPEs, other exceptions at a variety of locations (commands expecting arguments that don't get them). I'm working on some of this now. > Regd: throwing exception the idea was that server need not go crazy > checking > all possible bad arguments. It should at some point throw an exception and > terminate connection from an obviously bad client (or test program). This is in violation of the spec. There is no such thing as an "obviously bad client". The specification dictates the appropriate behavior for cases of malformed values. So we can't do that. > Excellent patches and bad bugs killed. Thank you. --Peter -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
