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

451-Error during read from data connection #205

Closed
PRR24 opened this issue Oct 26, 2021 · 13 comments · Fixed by #212
Closed

451-Error during read from data connection #205

PRR24 opened this issue Oct 26, 2021 · 13 comments · Fixed by #212

Comments

@PRR24
Copy link

PRR24 commented Oct 26, 2021

Describe the bug
There is a repeatable error during file upload in case of specific file size (around 45k, one specific file size is 45017 bytes) and network speed (fast).
If the file is smaller or larger, the error disappears (file content does not matter). If the network speed is slower, error also disappears. Reproduces with minimal code (new Client + access + uploadFrom), but as is location-specific, is probably very little use. Typically 32k gets uploaded before error appears (but sometimes less)

Possible racing condition?

Problem does NOT depend on:

  • file content
  • file name

Ubuntu 18.04
Node v12.22.1
basic-ftp 4.6.6

Would be happy to provide any additional data if requested.

Exception thrown:

FTPError: 451-Error during read from data connection
451-Transfer aborted
451 0.058 seconds (measured here), 0.54 Mbytes per second
    at FTPContext._onControlSocketData (node_modules/basic-ftp/dist/FtpContext.js:282:39)
    at TLSSocket.<anonymous> (node_modules/basic-ftp/dist/FtpContext.js:123:44)
    at TLSSocket.emit (events.js:314:20)
    at addChunk (_stream_readable.js:297:12)
    at readableAddChunk (_stream_readable.js:268:11)
    at TLSSocket.Readable.push (_stream_readable.js:213:10)
    at TLSWrap.onStreamRead (internal/stream_base_commons.js:188:23) {

Verbose log:

Connected to x.x.x.x:21 (No encryption)
< 220---------- Welcome to Pure-FTPd [privsep] [TLS] ----------
220-You are user number 9 of 500 allowed.
220-Local time is now 07:20. Server port: 21.
220-This is a private system - No anonymous login
220-IPv6 connections are also welcome on this server.
220 You will be disconnected after 30 minutes of inactivity.

> AUTH TLS
< 234 AUTH TLS OK.

Control socket is using: TLSv1.3
Login security: TLSv1.3
> USER x
< 331 User x OK. Password required

> PASS ###
< 230-User global permissions: DOWNLOAD (yes) UPLOAD (yes) [NEW (yes) RESUME (yes) OVERWRITE (yes) VIRUSES(no)] RENAME (yes) DELETE (yes) CHMOD (yes) MKDIR (yes) RMDIR (yes) CHDIR (yes) LIST (yes)

< 230 OK. Current restricted directory is /

> FEAT
< 211-Extensions supported:
 UTF8
 EPRT
 IDLE
 MDTM
 SIZE
 MFMT
 REST STREAM
 MLST type*;size*;sizd*;modify*;UNIX.mode*;UNIX.uid*;UNIX.gid*;unique*;
 MLSD
 PRET
 AUTH TLS
 PBSZ
 PROT
 TVFS
 ESTA
 PASV
 EPSV
 SPSV
 ESTP

< 211 End.

> TYPE I
< 200 TYPE is now 8-bit binary

> STRU F
< 200 F OK

> OPTS UTF8 ON
< 200 Always in UTF8 mode

> OPTS MLST type;size;modify;unique;unix.mode;unix.owner;unix.group;unix.ownername;unix.groupname;
< 200  MLST OPTS type;size;sizd;modify;UNIX.mode;UNIX.uid;UNIX.gid;unique;

> PBSZ 0
< 200 PBSZ=0

> PROT P
< 200 Data protection level set to "private"

> CWD /gfx/300x300
< 250 OK. Current directory is /gfx/300x300

Trying to find optimal transfer mode...
> EPSV
< 500 Unknown command

Transfer mode failed: "500 Unknown command", will try next.
> PASV
< 227 Entering Passive Mode (x,x,x,x,240,105)

Optimal transfer mode found.
> STOR 6u711d1f5yd4m3nu7g515k3i34bvon168731.png.part
< 150 Accepted data connection

Uploading to x.x.x.x:61545 (TLSv1.3)
< 451-Error during read from data connection

< 451-Transfer aborted

< 451 0.070 seconds (measured here), 459.00 Kbytes per second
@PRR24
Copy link
Author

PRR24 commented Oct 26, 2021

Tracked the issue down to transfer.ts

source.pipe(dataSocket).once("finish", () => {
    dataSocket.destroy() // Explicitly close/destroy the socket to signal the end.
    resolver.onDataDone(task)
})

It seems that the dataSocket gets destroyed before all the data is actually sent out by the dataSocket.

Should't it be

source.pipe(dataSocket).once("end", () => {

instead?

@PRR24
Copy link
Author

PRR24 commented Dec 1, 2021

I can confirm that after running the adjusted code (above) for a month now, the problem has not happened even once.

@olibp
Copy link

olibp commented Dec 22, 2021

Thanks! Had the same issue and this solved it.

@Ogginava
Copy link

Thanks! This fixed also solved my "FTPError: 450 Transfer aborted. Link to file server lost"

@theMK2k
Copy link

theMK2k commented Jun 19, 2022

Same issue, solved by that small patch by @PRR24.

So how do you people work with this as it doesn't seem to be integrated into basic-ftp?

@janwidmer
Copy link

I have the same problem, how can I apply the described workaround (besides changing the basic-ftp npm package code locally, which is not really a good idea..)?

@theMK2k
Copy link

theMK2k commented Jun 27, 2022

@janwidmer either

  • go the extra mile and create a fork and a new npm package
  • or version-pin the current basic-ftp release in your package.json, provide the patched dist_transfer.js file in your src and always overwrite the one in node_modules with yours

I currently work with the second option :<

@janwidmer
Copy link

@patrickjuchli would you accept a pr to fix this issue?

@patrickjuchli
Copy link
Owner

Sorry, haven't been aware of this issue for a long time and too busy to do anything here. I wanted to have a look first but if this does help all of you then let's just do this. Yes, will accept PR. Thanks all, and sorry again.

cjsewell added a commit to cjsewell/basic-ftp that referenced this issue Jul 3, 2022
Update event for closing/destroying the socket for uploads from finish to end

Fixes patrickjuchli#205
patrickjuchli pushed a commit that referenced this issue Jul 6, 2022
Update event for closing/destroying the socket for uploads from finish to end

Fixes #205
@uolot
Copy link

uolot commented Jul 8, 2022

If has been addressed in #212 but apparently it failed to build and there's no new release. The most recent release on npm is from a year ago.

@patrickjuchli patrickjuchli reopened this Jul 8, 2022
@patrickjuchli
Copy link
Owner

I'm working on a fix, I'll use pipeline instead of the current stream solution. I need to also revisit unit tests around transfers in general.

@patrickjuchli
Copy link
Owner

Should be fixed in 5.0.0.

@janwidmer
Copy link

Great, now it works, thanks for the new release.

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 a pull request may close this issue.

7 participants