Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NET-727 enable accessing options map for TFTP request packet #318

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vimukthiD
Copy link

relates to existing https://issues.apache.org/jira/browse/NET-727

This changes only includes the mapping of options map in TFTPRequestPacket which would be towards implementing RFC2347. For my use case only requirement was to get the blksize (RFC2348) from the request packet and would like to move on to try my luck with implementing the RFC2347.

@@ -56,6 +59,9 @@ public abstract class TFTPRequestPacket extends TFTPPacket {
/** The file name of the request. */
private final String fileName;

/** The option values */
private Map<String, String> options = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think options can be final.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for pointing that out , I've changed it to be final

Comment on lines 62 to 63
/** The option values */
private Map<String, String> options = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to rfc2347. A new error code with value 8 is defined to indicate that a transfer should be terminated due to option negotiation. I think you can safely add new err code to TFTPErrorPacket ;)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the TFTPErrorPacket to have the error code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants