Skip to content
This repository has been archived by the owner on Oct 15, 2019. It is now read-only.

active mode transfers work in ruby-2.3.0 #12

Merged
merged 3 commits into from
Jun 15, 2016
Merged

Conversation

wconrad
Copy link
Collaborator

@wconrad wconrad commented Jan 25, 2016

The symptom

Active mode file transfers do not work in Ruby 2.3.0, 2.2.3, and 2.2.4. The reason is that the PORT command is no longer sent.

With ruby 2.2, calling a file transfer method such as #gettextfile, when in active mode, causes this sequence of FTP commands:

TYPE A
PORT 127,0,0,1,175,206
RETR ascii_unix
TYPE I

With Ruby 2.3, the sequence is missing the PORT command:

TYPE A
200 Type set to A
RETR ascii_unix

The cause

As it necessarily does, double-bag-ftps is bound to the internals of Ruby's ftp class. In Ruby 2.3.0, one of those internals has changed in a way that breaks active mode file transfers: the #makeport method no longer automatically invokes #sendport. #sendport is what sends the PORT command to the server.

The fix

This patch explicitly issues the sendport command for ruby-2.3 or later.

With this patch, https://github.com/wconrad/ftpd/tree/ruby-2.3 passes its tests for these ruby versions:

  • 1.9.3
  • 2.1.5
  • 2.2.0
  • 2.2.1
  • 2.2.2
  • 2.2.3
  • 2.2.4
  • 2.3.0

@wconrad
Copy link
Collaborator Author

wconrad commented Apr 4, 2016

It appears that the changes to ftp.rb have been backported to 2.2.3, since this problem also occurs with ruby-2.2.3 and ruby-2.2.4. Until this problem is fixed, use ruby <= 2.2.2 if you need to do active mode transfers with double-bag-ftps.

@@ -109,6 +109,10 @@ def transfercmd(cmd, rest_offset = nil)
conn = ssl_socket(conn) # SSL connection now possible after cmd sent
else
sock = makeport
# Before ruby-2.3.0, makeport did a sendport automatically
if RUBY_VERSION >= "2.3"
Copy link

Choose a reason for hiding this comment

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

This version check needs to be appropriate for affected versions. Apparently 2.2.* series is also affected.

Copy link
Collaborator Author

@wconrad wconrad Apr 6, 2016

Choose a reason for hiding this comment

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

Good catch. Indeed, 2.1.0 through 2.1.2 not affected, but 2.1.3 and 2.1.4 are.

This is why I hate explicit version checks. That said, until I figure out how to do this without a version check, I'll add the appropriate versions of 2.1 to this if.

@wconrad
Copy link
Collaborator Author

wconrad commented Apr 6, 2016

Note: Although I added a line of code to make the match work with Ruby 1.8, I did not actually test this patch with Ruby 1.8. That's because I'm using the unit tests for the ftpd gem to test this patch, and the ftpd gem does not support Ruby 1.8.

@olbrich
Copy link

olbrich commented Apr 22, 2016

@bnix it would be great if this pull could get tested, merged, and released. Any chance of that happening soon?

@wconrad
Copy link
Collaborator Author

wconrad commented Apr 22, 2016

@olbrich Until this PR is accepted, you can try it by putting this in your Gemfile:

gem 'double-bag-ftps', '~> 0.1', git: "https://github.com/wconrad/double-bag-ftps.git", branch: "ruby-2.3"

(The ability to use a gem from someone's git repo has got to be one of Bundler's best features).

It would help to confirm my work if you could do that and report your results here.

@bnix
Copy link
Owner

bnix commented Apr 22, 2016

@wconrad would you like to be a collaborator on this repo? I'm not setup test against an FTPS server as I don't work on any projects using this gem anymore.

@wconrad
Copy link
Collaborator Author

wconrad commented Apr 22, 2016

@bnix Sure, sock it to me. I'll try not to break it.

@bnix
Copy link
Owner

bnix commented Apr 23, 2016

@wconrad you should now have push access.

@wconrad
Copy link
Collaborator Author

wconrad commented Apr 23, 2016

@bnix Thanks. Do you want me to prepare the release, merge it into master, and then you'll release the gem?

@bnix
Copy link
Owner

bnix commented Apr 28, 2016

@wconrad sounds good.

@basicBrogrammer
Copy link

What is the status on this pull request?

@wconrad
Copy link
Collaborator Author

wconrad commented Jun 15, 2016

@jcward10 I'm sorry for the delay. I'll try to get this branch straightened out so it can be merged.

This extends the previous patch, which worked for 2.3.  The changes to
ftp.rb in ruby 2.3 which broke double-bag-ftps have apparently been
back-ported to 2.2 starting with 2.2.3.

The `require 'rubygems'` is needed for this patch to work with Ruby
1.8.
@wconrad
Copy link
Collaborator Author

wconrad commented Jun 15, 2016

@bnix This is ready to go. The README is updated and the gemfile version bumped to 0.1.3. I think you should be able to publish the gem now.

Until the gem is published, anyone needing this fix can put this in their Gemfile to get the version of double-bag-ftps that has this fix:

gem 'double-bag-ftps',
    git: "https://github.com/bnix/double-bag-ftps.git",
    ref: "eca9efdecd242a2ee5e3f9cc2d8a2f7b4db6cfbb"

Or, to just get the latest unpublished version of double-bag-ftps (which may contain commits that came after this fix):

gem 'double-bag-ftps', git: "https://github.com/bnix/double-bag-ftps.git"

Once the new version of the gem is published, you can remove the git: and ref: options, run "bundle update," and you'll be back to using the published gem. Yay bundler!

@wconrad
Copy link
Collaborator Author

wconrad commented Aug 12, 2016

@bnix A new gem version is ready to release. The gem version in master has already been bumped to 0.1.3.

wconrad added a commit to wconrad/double-bag-ftps that referenced this pull request Nov 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants