Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.4
    • Fix Version/s: 2.0
    • Component/s: None
    • Labels:
      None

      Description

      Bug for attaching TFTP Server.

        Activity

        Hide
        Dan Armbrust added a comment -

        Attached is a full TFTP server implementation. Implements RFC 1350, and also http://www.faqs.org/rfcs/rfc1123.html section 4.2.3.1 which updates RFC 1350.

        This attachment also includes some JUnit tests - the JUnit tests verify the operation of the commons TFTPClient and the attached TFTP Server - uploads, downloads, block wrapping for large files, and access control on the server.

        These JUnit tests should prevent regression issues that have plagued the TFTP client code in the past.

        Show
        Dan Armbrust added a comment - Attached is a full TFTP server implementation. Implements RFC 1350, and also http://www.faqs.org/rfcs/rfc1123.html section 4.2.3.1 which updates RFC 1350. This attachment also includes some JUnit tests - the JUnit tests verify the operation of the commons TFTPClient and the attached TFTP Server - uploads, downloads, block wrapping for large files, and access control on the server. These JUnit tests should prevent regression issues that have plagued the TFTP client code in the past.
        Hide
        Rory Winston added a comment -

        Applied. Thanks, Dan.

        Show
        Rory Winston added a comment - Applied. Thanks, Dan.
        Hide
        Martin Oberhuber added a comment -

        The TFTPServer was removed again from recent commons net 1.5.0 candidates because it requires J2SE-1.4 whereas the rest of commons net 1.5.0 can do with J2SE-1.2 – the offending class is the use of SocketTimeoutException which was only added with J2SE-1.4

        Either the "Fix Version" 1.5 should be removed here or the TFTPServer should be re-added.

        Show
        Martin Oberhuber added a comment - The TFTPServer was removed again from recent commons net 1.5.0 candidates because it requires J2SE-1.4 whereas the rest of commons net 1.5.0 can do with J2SE-1.2 – the offending class is the use of SocketTimeoutException which was only added with J2SE-1.4 Either the "Fix Version" 1.5 should be removed here or the TFTPServer should be re-added.
        Hide
        Dan Armbrust added a comment -

        Is the SocketTimeoutException the only issue?

        I can prepare a new patch that works around that easy enough.

        Show
        Dan Armbrust added a comment - Is the SocketTimeoutException the only issue? I can prepare a new patch that works around that easy enough.
        Hide
        James Carman added a comment - - edited

        Isn't a full-fledged server outside the scope of Commons Net? Commons Net was supposed to be for accessing common internet services. To quote the first sentence of the Commons Net website:

        "Apache Commons Net implements the client side of many basic Internet protocols."

        I think this belongs in either its own project or as part of some other project.

        Show
        James Carman added a comment - - edited Isn't a full-fledged server outside the scope of Commons Net? Commons Net was supposed to be for accessing common internet services. To quote the first sentence of the Commons Net website: "Apache Commons Net implements the client side of many basic Internet protocols." I think this belongs in either its own project or as part of some other project.
        Hide
        Dan Armbrust added a comment -

        Well, I asked Rory that exact question, and he thought it would be fine to add if it didn't complicate things.

        A TFTP server is a natural fit with the TFTP client - and this one is implemented entirely on commons net - and there I haven't found any other reasonable open source java TFTP server in any other project. This isn't something like a Tomcat server. Its here, it works, and many people could use it. It's simple, and self contained. It doesn't bring in any other dependencies.

        Quite frankly, I think you need to keep it just so you can keep the JUnit tests that I included. Your TFTP client has been broken for years, because it was never properly tested after various patches were applied.

        With the new tests, and the server, you can at least be confident that the client doesn't have any regression failures.

        And I think that argument easily outweighs the the mission statement of only doing client protocols.

        Show
        Dan Armbrust added a comment - Well, I asked Rory that exact question, and he thought it would be fine to add if it didn't complicate things. A TFTP server is a natural fit with the TFTP client - and this one is implemented entirely on commons net - and there I haven't found any other reasonable open source java TFTP server in any other project. This isn't something like a Tomcat server. Its here, it works, and many people could use it. It's simple, and self contained. It doesn't bring in any other dependencies. Quite frankly, I think you need to keep it just so you can keep the JUnit tests that I included. Your TFTP client has been broken for years, because it was never properly tested after various patches were applied. With the new tests, and the server, you can at least be confident that the client doesn't have any regression failures. And I think that argument easily outweighs the the mission statement of only doing client protocols.
        Hide
        James Carman added a comment -

        A server implementation falls completely outside the scope of the original scope of the Commons Net proposal. A TFTP server should be a stand-alone component, IMHO (or part of a bigger project which supports standard internet protocols). What we're talking about is scope creep. Do you want to maintain HTTP servers or POP3, etc.? I would -1 this.

        Show
        James Carman added a comment - A server implementation falls completely outside the scope of the original scope of the Commons Net proposal. A TFTP server should be a stand-alone component, IMHO (or part of a bigger project which supports standard internet protocols). What we're talking about is scope creep. Do you want to maintain HTTP servers or POP3, etc.? I would -1 this.
        Hide
        Sebb added a comment -

        How about adding the server code to the test tree instead?

        This would have the advantage that the server can be tweaked if necessary, e.g. to generate error conditions.

        Show
        Sebb added a comment - How about adding the server code to the test tree instead? This would have the advantage that the server can be tweaked if necessary, e.g. to generate error conditions.
        Hide
        Dan Armbrust added a comment -

        Well, now that we have a Dilbert manager in the conversation, we can be sure that we wont let good code get in the way of a good mission statement.

        James, I really don't see why you are so worried. As far as I can tell from recent bugs, you haven't been maintaining anything WRT the TFTP code. So, I really don't see that adding one more class is going to increase your burden any. And the TFTP code has been quite seriously broken in recent releases, mostly because properly testing the code is a pain when you don't have a TFTP server set up. Now you have a TFTP server that is easy to use, and JUnit tests which verify operation of the server and the client.

        A POP3 server and an HTTP server are examples of much more complex servers. A TFTP server is hardly on the same scale. Like I mentioned before, what we have here is a single class that runs a TFTP server. It requires no changes to other commons net code, and no one is forcing anyone else to use it. Nobody is asking commons-net developers to write one. As far as maintenance concerns, I've usually found testable code is much easier to maintain that non-testable code. But hey, if you want to go back to having not-easily-testable tftp client code, go right ahead.

        As a "customer" of commons-net - this is where I would expect to find a TFTP server. Do you really thing that something as simple as a TFTP server should be a top-level apache project, on par with a HTTP server or a Tomcat server? Even the notion of a "commmons-tftp-server" project is pretty silly, given the size and simplicity of the thing. But if someone offers to set up such a sub-project for it, I'll put it there and maintain it. But I'm not going to do the legwork for it myself - I'll just go throw it out on google code if nothing else.

        In the end, its not my project. Do what you want. I'm willing to fix the compiling issue on 1.2 - and provide a new patch - it was a simple oversight. But I'm not going to waste any more time on it if you are just going to throw the code away, or bury it in your test code.

        Show
        Dan Armbrust added a comment - Well, now that we have a Dilbert manager in the conversation, we can be sure that we wont let good code get in the way of a good mission statement. James, I really don't see why you are so worried. As far as I can tell from recent bugs, you haven't been maintaining anything WRT the TFTP code. So, I really don't see that adding one more class is going to increase your burden any. And the TFTP code has been quite seriously broken in recent releases, mostly because properly testing the code is a pain when you don't have a TFTP server set up. Now you have a TFTP server that is easy to use, and JUnit tests which verify operation of the server and the client. A POP3 server and an HTTP server are examples of much more complex servers. A TFTP server is hardly on the same scale. Like I mentioned before, what we have here is a single class that runs a TFTP server. It requires no changes to other commons net code, and no one is forcing anyone else to use it. Nobody is asking commons-net developers to write one. As far as maintenance concerns, I've usually found testable code is much easier to maintain that non-testable code. But hey, if you want to go back to having not-easily-testable tftp client code, go right ahead. As a "customer" of commons-net - this is where I would expect to find a TFTP server. Do you really thing that something as simple as a TFTP server should be a top-level apache project, on par with a HTTP server or a Tomcat server? Even the notion of a "commmons-tftp-server" project is pretty silly, given the size and simplicity of the thing. But if someone offers to set up such a sub-project for it, I'll put it there and maintain it. But I'm not going to do the legwork for it myself - I'll just go throw it out on google code if nothing else. In the end, its not my project. Do what you want. I'm willing to fix the compiling issue on 1.2 - and provide a new patch - it was a simple oversight. But I'm not going to waste any more time on it if you are just going to throw the code away, or bury it in your test code.
        Hide
        James Carman added a comment -

        Mr. Armbrust,

        You're correct. I do not work on the Commons Net project, or at least I haven't in the past. But, Commons Net is an Apache Software Foundation project and the Apache Software Foundation is all about community, so I am entitled to my opinion as a member of that community. Belittling ("Dilbert manager") someone for providing their opinion is not very helpful to the community. So, let's try to keep the personal attacks out of it and stay on topic, please.

        I like the idea of having a TFTP server. I just don't think it belongs in Commons Net. Commons Net is all about client code. I have no problem with this code going into either another project or being one all on its own. Commons Net can depend on that project as a test-scoped dependency. If there's no proper home for the TFTP server, then I like sebb's idea of putting it in the test code.

        Apache Commons projects must strive to stick to their original scope. If the TFTP server goes into the core codebase, then somebody is going to use it. When that person uses it, they're going to find something wrong with it or see some way that it can be enhanced. Then, we get in the business of implementing and maintaining a proper TFTP server implementation. That's not what Commons Net (or Apache Commons overall for that matter) is intended for.

        Show
        James Carman added a comment - Mr. Armbrust, You're correct. I do not work on the Commons Net project, or at least I haven't in the past. But, Commons Net is an Apache Software Foundation project and the Apache Software Foundation is all about community, so I am entitled to my opinion as a member of that community. Belittling ("Dilbert manager") someone for providing their opinion is not very helpful to the community. So, let's try to keep the personal attacks out of it and stay on topic, please. I like the idea of having a TFTP server. I just don't think it belongs in Commons Net. Commons Net is all about client code. I have no problem with this code going into either another project or being one all on its own. Commons Net can depend on that project as a test-scoped dependency. If there's no proper home for the TFTP server, then I like sebb's idea of putting it in the test code. Apache Commons projects must strive to stick to their original scope. If the TFTP server goes into the core codebase, then somebody is going to use it. When that person uses it, they're going to find something wrong with it or see some way that it can be enhanced. Then, we get in the business of implementing and maintaining a proper TFTP server implementation. That's not what Commons Net (or Apache Commons overall for that matter) is intended for.
        Hide
        Niall Pemberton added a comment -

        I agree with James - personal comments just detract.

        But my view of the ASF is its a do-ocracy rather than about rigid rules. Since Rory has been doing all the work on Net and saw fit to include it and it is a trivial implementation then I don't see any problem. Your point about maintaining it is true of any code we choose to release and again since Rory accepted it, I see no reason to assume he doesn't intend to fix bugs. How comprehensive the impelmentation is or becomes is down to the Net devs - users may ask for or want more - but its within the Net devs control on what they choose to provide and to set the scope and limitations of the implementation provided.

        Anyway, thats my 2 cents from someone else who hasn't fixed a line of Net code.

        Show
        Niall Pemberton added a comment - I agree with James - personal comments just detract. But my view of the ASF is its a do-ocracy rather than about rigid rules. Since Rory has been doing all the work on Net and saw fit to include it and it is a trivial implementation then I don't see any problem. Your point about maintaining it is true of any code we choose to release and again since Rory accepted it, I see no reason to assume he doesn't intend to fix bugs. How comprehensive the impelmentation is or becomes is down to the Net devs - users may ask for or want more - but its within the Net devs control on what they choose to provide and to set the scope and limitations of the implementation provided. Anyway, thats my 2 cents from someone else who hasn't fixed a line of Net code.
        Hide
        Dan Armbrust added a comment -

        James,

        I meant the Dilbert comment as light-hearted, rather than attack. I need to remember that tone and inflection don't carry across text very well

        I said it because I took your "-1" comment as a "this isn't happening on my watch - I'm not allowing it" type of statement, rather than your opinion that it shouldn't happen if it were solely up to you.

        I understand your concern over maintaining a proper server. That said, I'm fairly certain that the provided server already is a proper TFTP server implementation. If someone uses it, and discovers a bug, no one says you have to fix it. You can always just let it be, until some other volunteer comes along (like me) and fixes the bug.

        But thats just my opinion.

        Show
        Dan Armbrust added a comment - James, I meant the Dilbert comment as light-hearted, rather than attack. I need to remember that tone and inflection don't carry across text very well I said it because I took your "-1" comment as a "this isn't happening on my watch - I'm not allowing it" type of statement, rather than your opinion that it shouldn't happen if it were solely up to you. I understand your concern over maintaining a proper server. That said, I'm fairly certain that the provided server already is a proper TFTP server implementation. If someone uses it, and discovers a bug, no one says you have to fix it. You can always just let it be, until some other volunteer comes along (like me) and fixes the bug. But thats just my opinion.
        Hide
        James Carman added a comment -

        Niall,

        While I agree with the general do-ocracy idea (and the folks who have worked on Net have done a great job), I don't necessarily agree that projects should be allowed to just change their scope whenever they choose. Part of the proposal to join Apache Commons is the scope of the project. Projects are allowed in after a vote by the PMC based on the proposal. There should be some assurance that the project is going to stick to that proposed scope. What happens if a project decides to change its scope and that new scope would have made it fail to enter Commons in the first place because it violates the charter?

        Now, I'm not saying that's what's going on here with Net at all. I wouldn't really have a problem with Net changing its scope to include server implementations, but it hasn't been officially proposed.

        Another point to consider is that once you start adding in trivial server implementations for some protocols, then it stands to reason that folks might expect that for some of the other protocols also. Now, we don't always have to do what we're asked, but we should try to have some consistency and having one server implementation for one specific protocol and none of the others isn't really consistent (IMHO).

        Show
        James Carman added a comment - Niall, While I agree with the general do-ocracy idea (and the folks who have worked on Net have done a great job), I don't necessarily agree that projects should be allowed to just change their scope whenever they choose. Part of the proposal to join Apache Commons is the scope of the project. Projects are allowed in after a vote by the PMC based on the proposal. There should be some assurance that the project is going to stick to that proposed scope. What happens if a project decides to change its scope and that new scope would have made it fail to enter Commons in the first place because it violates the charter? Now, I'm not saying that's what's going on here with Net at all. I wouldn't really have a problem with Net changing its scope to include server implementations, but it hasn't been officially proposed. Another point to consider is that once you start adding in trivial server implementations for some protocols, then it stands to reason that folks might expect that for some of the other protocols also. Now, we don't always have to do what we're asked, but we should try to have some consistency and having one server implementation for one specific protocol and none of the others isn't really consistent (IMHO).
        Hide
        Niall Pemberton added a comment -

        Logically you're right and seeing a rash of server implementations here would not be a good idea. But I don't think thats whats going on here or likely to happen - one trivial impl. thats useful for people to test and IMO violate is too strong a word to characterize whats being proposed. Sitting from my armchair IMO Dan has made a good case for adding this one impl. here and just because we accept that doesn't mean we can't say "hey stop, lets not do this" when someone proposes a second. Precedent has some weight to a certain point, but this is not the legal system and the PMC can make whatever decisions it likes - including preventing further implementations despite accepting this one.

        Show
        Niall Pemberton added a comment - Logically you're right and seeing a rash of server implementations here would not be a good idea. But I don't think thats whats going on here or likely to happen - one trivial impl. thats useful for people to test and IMO violate is too strong a word to characterize whats being proposed. Sitting from my armchair IMO Dan has made a good case for adding this one impl. here and just because we accept that doesn't mean we can't say "hey stop, lets not do this" when someone proposes a second. Precedent has some weight to a certain point, but this is not the legal system and the PMC can make whatever decisions it likes - including preventing further implementations despite accepting this one.
        Hide
        Rory Winston added a comment -

        Hi guys

        Thanks for all of your comments. There is a very good point here, in that including server code is beyond the stated remit of the project, and including it may just end up being confusing. Having said that, here is why I decided to include it:

        1. It is a pretty simple and small implementation, that doesnt create any extra dependencies or have any effect on existing code.
        2. I liked the contribution (thanks Dan) and did want to make use of it, but I couldnt think of a good alternative home for it outside [net]. I also thought it may be too much work to set up a separate project for basically a one-class contribution.
        3. As somebody pointed out earlier, I don't want to start advertising [net] as server code implementations suddenly. I have enough problems keeping up with the client part . But this small addition may help to keep the TFTP client codebase reliable, as we've had some issues with it in the past.

        James (and others) have suggested moving the TFTPServer code to the test src tree. I think this is A Fine Idea, as having it in our source tree implies that it is a test dependency (which it is right now), and thus we can still be seen to be staying within the bounds of the project scope. If no-one has any objections, that's what I'll do (as soon as I sort out my SVN client props - sorry Niall ).

        Thanks for all of the interest and discussion around this - it really shows that you guys have a real commitment to quality. Many thanks to Sebb and Niall for all of their help with the release stuff so far.

        Cheers
        Rory

        Show
        Rory Winston added a comment - Hi guys Thanks for all of your comments. There is a very good point here, in that including server code is beyond the stated remit of the project, and including it may just end up being confusing. Having said that, here is why I decided to include it: 1. It is a pretty simple and small implementation, that doesnt create any extra dependencies or have any effect on existing code. 2. I liked the contribution (thanks Dan) and did want to make use of it, but I couldnt think of a good alternative home for it outside [net] . I also thought it may be too much work to set up a separate project for basically a one-class contribution. 3. As somebody pointed out earlier, I don't want to start advertising [net] as server code implementations suddenly. I have enough problems keeping up with the client part . But this small addition may help to keep the TFTP client codebase reliable, as we've had some issues with it in the past. James (and others) have suggested moving the TFTPServer code to the test src tree. I think this is A Fine Idea, as having it in our source tree implies that it is a test dependency (which it is right now), and thus we can still be seen to be staying within the bounds of the project scope. If no-one has any objections, that's what I'll do (as soon as I sort out my SVN client props - sorry Niall ). Thanks for all of the interest and discussion around this - it really shows that you guys have a real commitment to quality. Many thanks to Sebb and Niall for all of their help with the release stuff so far. Cheers Rory
        Hide
        Rory Winston added a comment -

        Just one last thing:

        Dan, moving the server code to the test src tree won't be burying it at all - it's just attempting to keep the boundaries of the project clearer. In fact, it may just be more about semantics at the end of the day - I would still like to make the server class available in generated .jars so clients could use it if desired, which is exactly the same situation as right now.

        Show
        Rory Winston added a comment - Just one last thing: Dan, moving the server code to the test src tree won't be burying it at all - it's just attempting to keep the boundaries of the project clearer. In fact, it may just be more about semantics at the end of the day - I would still like to make the server class available in generated .jars so clients could use it if desired, which is exactly the same situation as right now.
        Hide
        Dan Armbrust added a comment -

        As long as it is in the distribution jar, I'll be happy, since I won't have to keep my own copy on the side anymore.

        Did you want a fix for the jdk 1.2 issue - so it can go into the 1.5 build? Or is that process to far along already?

        Will you be releasing a 2.0 build along side the 1.5 build? I'm perfectly happy moving up to a 2.0 release if one is going to be available.

        Show
        Dan Armbrust added a comment - As long as it is in the distribution jar, I'll be happy, since I won't have to keep my own copy on the side anymore. Did you want a fix for the jdk 1.2 issue - so it can go into the 1.5 build? Or is that process to far along already? Will you be releasing a 2.0 build along side the 1.5 build? I'm perfectly happy moving up to a 2.0 release if one is going to be available.
        Hide
        Rory Winston added a comment -

        Hey Dan

        yes, i plan to release 2.0 and 1.5 simultaneously. It might be cleaner to keep the TFTP server in 2.0 for now.

        Cheers

        Show
        Rory Winston added a comment - Hey Dan yes, i plan to release 2.0 and 1.5 simultaneously. It might be cleaner to keep the TFTP server in 2.0 for now. Cheers
        Hide
        James Carman added a comment -

        Instead of having test classes in your distribution jar, I'd suggest you have Maven create a "test jar" for your project:

        http://maven.apache.org/guides/mini/guide-attached-tests.html

        This way, if someone wants to use your TFTPServer implementation, they can declare this dependency:

        <dependency>
        <groupId>org.apache.commons</groupId>
        <artifactId>commons-net</artifactId>
        <version>2.0</version>
        <classifier>tests</classifier>
        </dependency>

        Show
        James Carman added a comment - Instead of having test classes in your distribution jar, I'd suggest you have Maven create a "test jar" for your project: http://maven.apache.org/guides/mini/guide-attached-tests.html This way, if someone wants to use your TFTPServer implementation, they can declare this dependency: <dependency> <groupId>org.apache.commons</groupId> <artifactId>commons-net</artifactId> <version>2.0</version> <classifier> tests </classifier> </dependency>
        Hide
        Rory Winston added a comment -

        Hi James

        Thanks! I was looking for a neat way to do this. I'll document this when it's done.

        Cheers
        Rory

        Show
        Rory Winston added a comment - Hi James Thanks! I was looking for a neat way to do this. I'll document this when it's done. Cheers Rory

          People

          • Assignee:
            Unassigned
            Reporter:
            Dan Armbrust
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development