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.

Sign up for GitHub

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

FtpGateway: Unexpected ABORTED suffixes in %Ss #267

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

FtpGateway: Unexpected ABORTED suffixes in %Ss #267

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2759372
FtpGateway: Unexpected ABORTED suffixes in %Ss
eduard-bagdasaryan Oct 2, 2024
74cf59a
Take adaptation into account before completeForwarding()
eduard-bagdasaryan Oct 3, 2024
e322b47
Complete StoreEntry from FwdState::markStoredReplyAsWhole()
eduard-bagdasaryan Oct 21, 2024
43fbb6d
Add missing markStoredReplyAsWhole() in FTP code
eduard-bagdasaryan Oct 23, 2024
03e4d09
Undone an out-of-scope change
eduard-bagdasaryan Oct 23, 2024
5ebd652
Made Ftp::DataChannel::payloadSeen unsigned
eduard-bagdasaryan Oct 27, 2024
3b1c523
Undone Ftp::Gateway::completeForwarding() fix
eduard-bagdasaryan Oct 27, 2024
abe83b9
Adjusted a variable type and removed an always-true check
eduard-bagdasaryan Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/clients/FtpClient.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,14 @@ Ftp::DataChannel::addr(const Ip::Address &import)
port = import.port();
}

void
Ftp::DataChannel::appended(const size_t size)
{
readBuf->appended(size);
payloadSeen += size;
debugs(9, 5, size << " bytes to readBuf, payloadSeen: " << payloadSeen);
}

/* Ftp::Client */

Ftp::Client::Client(FwdState *fwdState):
Expand Down Expand Up @@ -979,8 +987,7 @@ Ftp::Client::dataRead(const CommIoCbParams &io)
}

if (io.flag == Comm::OK && io.size > 0) {
debugs(9, 5, "appended " << io.size << " bytes to readBuf");
data.readBuf->appended(io.size);
data.appended(io.size);
#if USE_DELAY_POOLS
DelayId delayId = entry->mem_obj->mostBytesAllowed();
delayId.bytesIn(io.size);
Expand Down
6 changes: 6 additions & 0 deletions src/clients/FtpClient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,17 @@ class DataChannel: public Ftp::Channel

void addr(const Ip::Address &addr); ///< import host and port

/// updates counters after this number of bytes have been added to readBuf
void appended(size_t size);

public:
MemBuf *readBuf;
char *host;
unsigned short port;
bool read_pending;

/// amount of message payload/body received so far
int64_t payloadSeen = 0;
};

/// FTP client functionality shared among FTP Gateway and Relay clients.
Expand Down
8 changes: 7 additions & 1 deletion src/clients/FtpGateway.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,11 @@ Ftp::Gateway::processReplyBody()

entry->flush();

if (theSize >= 0 && data.payloadSeen >= theSize) {
markParsedVirginReplyAsWhole("whole virgin body");
completeForwarding();
Copy link
Author

Choose a reason for hiding this comment

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

This fixes 'ABORTED' for ftp 'GET' transactions. We cannot add this to ftpReadTransferDone() similarly as to ftpWriteTransferDone() for 'PUT' transactions because for 'GET' transactions 226/250 response may come after Squid sends the whole downloaded file to the client (and client may have closed the connection before 226/250). Note that this does not happen for 'PUT' transactions, because ftpWriteTransferDone() initiates the response itself, i.e., the client receives the response only after Squid gets 226/250.

}

maybeReadVirginBody();
}

Expand Down Expand Up @@ -2274,6 +2279,7 @@ ftpWriteTransferDone(Ftp::Gateway * ftpState)
ftpState->entry->timestampsSet(); /* XXX Is this needed? */
ftpState->markParsedVirginReplyAsWhole("ftpWriteTransferDone code 226 or 250");
ftpSendReply(ftpState);
ftpState->completeForwarding();
}

static void
Expand Down Expand Up @@ -2639,7 +2645,7 @@ Ftp::Gateway::completeForwarding()
{
if (fwd == nullptr || flags.completed_forwarding) {
debugs(9, 3, "avoid double-complete on FD " <<
(ctrl.conn ? ctrl.conn->fd : -1) << ", Data FD " << data.conn->fd <<
(ctrl.conn ? ctrl.conn->fd : -1) << ", Data FD " << (data.conn ? data.conn->fd : -1) <<
", this " << this << ", fwd " << fwd);
return;
}
Expand Down