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

Enhancing the FTP protocol #40

Merged
merged 17 commits into from
Oct 22, 2023
Merged

Conversation

RomanRII
Copy link
Contributor

@RomanRII RomanRII commented Sep 24, 2023

What was done

  • Modified nxc/protocols/ftp.py
    • Modified the --ls flag to allow for listing the current directory and sub-directories. Default now lists .. If an argument is provided, it will list the provided sub-directory
    • Added the --get flag to download a file on the server. If the file exists and is successfully downloaded, it will be written to the users cwd with the remote file's filename.
    • Added the --put flag to upload files onto the server.
  • Modified nxc/protocols/ftp/proto_args.py to reflect the added features
    • Modified the --ls flag to allow for a default directory listing (.) or use a provided directory
    • Added the --get and --put flags
  • Modified nxc/protocols/ftp.py#L83 to comply with RFC 1635

Test environment used

  • During testing, different permissions were used to account for edge cases.
from pyftpdlib.authorizers import DummyAuthorizer
from pyftpdlib.handlers import FTPHandler
from pyftpdlib.servers import FTPServer

def start_ftp_server():
    # Instantiate an authorizer object responsible for managing authentication and authorization
    authorizer = DummyAuthorizer()

    # Allow anonymous access with read-only permissions
    authorizer.add_anonymous("./FTP_Folder/", perm="erlw")

    # Instantiate an FTP handler object and link it to the authorizer
    handler = FTPHandler
    handler.authorizer = authorizer

    # Create the FTP server on the desired address and port, and link it to the handler
    server = FTPServer(("127.0.0.1", 21), handler)

    # Start the FTP server
    server.serve_forever()

if __name__ == "__main__":
    start_ftp_server()

… Modified --ls

Modified nxc/protocols/ftp.py and added --get and --put functionality. Modified --ls functionality
@NeffIsBack NeffIsBack added the enhancement New feature or request label Sep 24, 2023
@NeffIsBack NeffIsBack added this to the v1.1.0 milestone Sep 24, 2023
…g to upload it to a ftp server using --put
@RomanRII
Copy link
Contributor Author

RomanRII commented Sep 24, 2023

RFC 1635 Compliance

Traditionally, this special anonymous user account accepts any string
as a password, although it is common to use either the password
"guest" or one's electronic mail (e-mail) address.  Some archive
sites now explicitly ask for the user's e-mail address and will not
allow login with the "guest" password.  Providing an e-mail address
is a courtesy that allows archive site operators to get some idea of
who is using their services.

image

--ls

  • Standard usage
    image
  • If permissions are not granted
    image

--get

  • Standard usage
    image
  • If the file does not exist
    image

--put

  • Standard usage
    image
  • If a local file does not exist
    image
  • Permissions error
    image

@RomanRII
Copy link
Contributor Author

Couple things to mind about this merge, it currently doesn't allow the following:

--put

  • If a user attempts to upload a file like --put example.txt ., it will fail with Failed to upload file. Response: (550 Is a directory.)

The user will need to specify the filename or dir + filename to save to
--put example.txt example.txt or --put example.txt Sub/Dir/example.txt

If this current implementation is not prefered, let me know and I can make some adjustments to allow users to upload using either . or Sub/ and upload to the directory using the local_name

@Marshall-Hallenbeck Marshall-Hallenbeck changed the base branch from main to develop October 2, 2023 21:44
@Marshall-Hallenbeck
Copy link
Collaborator

@RomanRII can you add these new flags to the e2e tests commands file under the FTP section (tests/e2e_commands.txt)?

@RomanRII
Copy link
Contributor Author

RomanRII commented Oct 3, 2023

@Marshall-Hallenbeck you got it. will get that done

@RomanRII
Copy link
Contributor Author

RomanRII commented Oct 3, 2023

Copy link
Contributor

@NeffIsBack NeffIsBack left a comment

Choose a reason for hiding this comment

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

Really good PR! Clean code and very nice documentation of the changes! Love to see that

nxc/protocols/ftp/proto_args.py Outdated Show resolved Hide resolved
nxc/protocols/ftp.py Outdated Show resolved Hide resolved
@NeffIsBack NeffIsBack added the reviewed code Label for when a static code review was done label Oct 15, 2023
RomanRII and others added 5 commits October 15, 2023 08:12
Signed-off-by: Roman Rivas II <74742067+RomanRII@users.noreply.github.com>
Signed-off-by: Roman Rivas II <74742067+RomanRII@users.noreply.github.com>
Copy link
Collaborator

@Marshall-Hallenbeck Marshall-Hallenbeck left a comment

Choose a reason for hiding this comment

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

Reviewed and tested this. It all works. Great code and feature, thank you!

@Marshall-Hallenbeck Marshall-Hallenbeck merged commit e9313c6 into Pennyw0rth:develop Oct 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers reviewed code Label for when a static code review was done tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants