Skip to content

Fix Transmit client treating a successful PASV response as failure#85

Open
michalc wants to merge 1 commit into
aio-libs:masterfrom
michalc:transmit-pasv-fix
Open

Fix Transmit client treating a successful PASV response as failure#85
michalc wants to merge 1 commit into
aio-libs:masterfrom
michalc:transmit-pasv-fix

Conversation

@michalc

@michalc michalc commented Nov 1, 2018

Copy link
Copy Markdown
Contributor

This fixes PASV connections on the Transmit client, that doesn't seem to handle the multiline response. An example transcript is below.

**********

Date/Time: 2018-11-01 16:37:39 +0000

1: Transmit 5.2.1 (x86_64) Session Transcript [Version 10.13.5 (Build 17F77)] (01/11/2018, 16:37)
1: LibNcFTP 3.2.3 (July 23, 2009) compiled for UNIX
1: 220: welcome
1: Connected to 127.0.01.
1: Cmd: USER myu
1: 331: password required
1: Cmd: PASS xxxxxxxx
1: 230: normal login
1: Cmd: TYPE A
1: 200: 
1: Logged in to 127.0.01 as myu.
1: Cmd: SYST
1: 215: UNIX Type: L8
1: Cmd: FEAT
1: 502: 'feat' not implemented
1: Cmd: PWD
1: 257: "/"
1: Cmd: PASV
1: Cannot parse PASV response: listen socket created
1: 227: listen socket created
1:      (0,0,0,0,15,161)
1: Passive mode refused.
1: Connection falling back to port (PORT) mode.
1: Cmd: PORT 127,0,0,1,243,198
1: 502: 'port' not implemented
1: Cmd: PORT 127,0,0,1,243,199
1: 502: 'port' not implemented
1: Cmd: PORT 127,0,0,1,243,200
1: 502: 'port' not implemented
1: Cmd: PORT 127,0,0,1,243,201
1: 502: 'port' not implemented
1: Canceling…
1: Disconnecting from server…
1: Cmd: QUIT
1: 221: bye

@pohmelie

pohmelie commented Nov 1, 2018

Copy link
Copy Markdown
Collaborator

I don't get what this solves and why multiline is a problem?

@michalc

michalc commented Nov 1, 2018

Copy link
Copy Markdown
Contributor Author

@pohmelie I have amended the PR description with more details

@codecov

codecov Bot commented Nov 1, 2018

Copy link
Copy Markdown

Codecov Report

Merging #85 into master will decrease coverage by 0.05%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #85      +/-   ##
==========================================
- Coverage   94.04%   93.98%   -0.06%     
==========================================
  Files           7        7              
  Lines        1695     1695              
==========================================
- Hits         1594     1593       -1     
- Misses        101      102       +1
Impacted Files Coverage Δ
aioftp/server.py 96.92% <66.66%> (-0.15%) ⬇️

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 80089a6...9b294bd. Read the comment docs.

@pohmelie

pohmelie commented Nov 1, 2018

Copy link
Copy Markdown
Collaborator

I think at first you should try to get feedback from Transmit devs via e-mail (https://library.panic.com/transmit/). Sadly, their GOLD STANDARD app have no bugtracker.

227-listen socket created
227 (127,0,0,1,154,21)

Response is valid and works fine with all tools I tried.

@michalc

michalc commented Nov 1, 2018

Copy link
Copy Markdown
Contributor Author

I have now emailed them, thanks.

@pohmelie

Copy link
Copy Markdown
Collaborator

@michalc any news?

@tomharvey

Copy link
Copy Markdown

I'm getting the same error - but not just in Transmit (also seeing an error while using Cyberduck). Also, the PR from @michalc doesn't seem to fix the issue; still Cannot parse PASV response: listen socket created.

Maybe this should be an issue instead of in here?

I am using 0.0.0.0 as the bind IP, and then connecting to 127.0.0.1 - is this maybe related to passive not liking the local IP addresses? I can investigate more next weekend..

@pohmelie

pohmelie commented Mar 1, 2020

Copy link
Copy Markdown
Collaborator

I can investigate more next weekend

It will be good if you have time of course.

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.

3 participants