Skip to content

Commit

Permalink
FtpGateway: Unexpected ABORTED suffixes in %Ss
Browse files Browse the repository at this point in the history
While downloading/uploading files using HTTP GET/PUT requests with ftp
URI scheme, Squid completes these transactions successfully but
sometimes logs the ABORTED suffix, e.g., TCP_MISS_ABORTED.  This happens
because Squid postpones completing the corresponding StoreEntry until
getting 221 reply from FTP server, even though the server has sent all
response bytes already. So if the client closes the client-to-Squid
connection before that, the cleanup code in
ConnStateData::terminateAll() finishes the transaction which is logged
as 'ABORTED' because the StoreEntry is still in the STORE_PENDING state.

Now we complete the entry, calling Ftp::Gateway::completeForwarding() as
soon as Squid got all FTP response bytes.
  • Loading branch information
eduard-bagdasaryan committed Oct 2, 2024
1 parent 620efbd commit 2759372
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
11 changes: 9 additions & 2 deletions src/clients/FtpClient.cc
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
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
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();
}

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

0 comments on commit 2759372

Please sign in to comment.