Skip to content

Conversation

@drakkan
Copy link
Collaborator

@drakkan drakkan commented Dec 17, 2020

Thank you!

@codecov
Copy link

codecov bot commented Dec 17, 2020

Codecov Report

Merging #203 (232b3a6) into master (fdb9611) will increase coverage by 0.76%.
The diff coverage is 77.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #203      +/-   ##
==========================================
+ Coverage   72.62%   73.39%   +0.76%     
==========================================
  Files           9        9              
  Lines        1001     1060      +59     
==========================================
+ Hits          727      778      +51     
- Misses        194      202       +8     
  Partials       80       80              
Impacted Files Coverage Δ
server.go 56.06% <ø> (ø)
handle_files.go 78.19% <75.00%> (+1.14%) ⬆️
client_handler.go 63.21% <100.00%> (+0.97%) ⬆️
handle_misc.go 95.58% <100.00%> (+0.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fdb9611...232b3a6. Read the comment docs.

Copy link
Owner

@fclairamb fclairamb left a comment

Choose a reason for hiding this comment

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

LGTM. You're well on your way to building the server lib that supports the most features of the FTP protocol!

I didn't know the COMB command. That's actually a pretty useful one.

@probot-auto-merge probot-auto-merge bot merged commit 695e05f into fclairamb:master Dec 18, 2020
@drakkan
Copy link
Collaborator Author

drakkan commented Dec 18, 2020

This is a proprietary command supported in several commercial FTP server such as Titan, GlobalSCAPE etc. It is useful for local files but useless for S3/GCS/Azure backends.

I have almost finished my TODO. The last thing is ABOR support. This will change the server logic and should be reviewed and tested carefully.

I think for some optional interface could be useful to return a well know error and handle the request ourself in this case.

I mean something similar to this: if the ContentTyper interface implementation returns ErrNotImplemented the library will use its internal implementation. In our case it could be useful for hash, for example:

  • you implement the Hasher interface
  • if the hash is md5 and the backend is gdrive you fetch the hash in your application
  • if you return ErrNotImplemented ftpserverlib will calculate the hash itself so you don't have to duplicate the library code in your application to calculate the hash for local fils.

Other minor things that I could contribute in future:

  • ModeZ
  • Allow to customize login ok and login fail messages (multilines)
  • Ensure that the active data connection will be started from the same IP as the control connection (configurable)
  • CISD command support

@drakkan drakkan deleted the comb branch February 3, 2021 15:48
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