Skip to content

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

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

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

Conversation

@michalc
Copy link
Copy Markdown
Contributor

@michalc michalc commented Nov 1, 2018

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
Copy link
Copy Markdown
Collaborator

pohmelie commented Nov 1, 2018

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

@michalc
Copy link
Copy Markdown
Contributor Author

michalc commented Nov 1, 2018

@pohmelie I have amended the PR description with more details

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 1, 2018

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
Copy link
Copy Markdown
Collaborator

pohmelie commented Nov 1, 2018

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
Copy link
Copy Markdown
Contributor Author

michalc commented Nov 1, 2018

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
Copy link
Copy Markdown
Collaborator

pohmelie commented Mar 1, 2020

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