[squid-dev] [PATCH] squid-3.5: fix for errors when receiving a non-existent file via ftp
Amos Jeffries
squid3 at treenet.co.nz
Sun Oct 11 15:56:13 UTC 2015
Attached is a cleaned version of the patch. It now meets the source
coding guidelines, and applies cleanly to trunk / Squid-4.
I took the opportunity to remove two useless if() conditions - one was
duplicate checking a value, the other for values in a pointer that was
just new()'d.
Logic looks mostly okay, but this part in the middle of
Ftp::Gateway::loginFailed() is rather odd:
- // any other problems are general falures.
- if (!err) {
- ftpFail(this);
+ if (!err)
return;
- }
The original behaviour was to output an error and perform the ftpQuit()
sequence. Now it appears to just halt without doing anything.
Is that behaviour change intentional? I suspect the correct behaviour is
to remove that if()-condition entirely, but I'm unsure.
Amos
-------------- next part --------------
=== modified file 'src/clients/FtpClient.cc'
--- src/clients/FtpClient.cc 2015-08-30 00:26:47 +0000
+++ src/clients/FtpClient.cc 2015-10-11 15:21:07 +0000
@@ -225,75 +225,86 @@
debugs(9, 3, "closing FTP data FD " << data.conn->fd << ", this " << this);
data.close();
}
debugs(9, 3, "FTP ctrl and data connections closed. this " << this);
}
/**
* Did we close all FTP server connection(s)?
*
\retval true Both server control and data channels are closed. And not waiting for a new data connection to open.
\retval false Either control channel or data is still active.
*/
bool
Ftp::Client::doneWithServer() const
{
return !Comm::IsConnOpen(ctrl.conn) && !Comm::IsConnOpen(data.conn);
}
void
-Ftp::Client::failed(err_type error, int xerrno)
+Ftp::Client::failed(err_type error, int xerrno, ErrorState *err)
{
debugs(9, 3, "entry-null=" << (entry?entry->isEmpty():0) << ", entry=" << entry);
const char *command, *reply;
- const Http::StatusCode httpStatus = failedHttpStatus(error);
- ErrorState *const ftperr = new ErrorState(error, httpStatus, fwd->request);
+ ErrorState *ftperr;
+
+ if (err) {
+ debugs(9, 6, "error=" << err->type << ", code=" << xerrno <<
+ ", status=" << err->httpStatus);
+ error = err->type;
+ ftperr = err;
+ } else {
+ Http::StatusCode httpStatus = failedHttpStatus(error);
+ ftperr = new ErrorState(error, httpStatus, fwd->request);
+ }
+
ftperr->xerrno = xerrno;
ftperr->ftp.server_msg = ctrl.message;
ctrl.message = NULL;
if (old_request)
command = old_request;
else
command = ctrl.last_command;
if (command && strncmp(command, "PASS", 4) == 0)
command = "PASS <yourpassword>";
if (old_reply)
reply = old_reply;
else
reply = ctrl.last_reply;
if (command)
ftperr->ftp.request = xstrdup(command);
if (reply)
ftperr->ftp.reply = xstrdup(reply);
- fwd->request->detailError(error, xerrno);
- fwd->fail(ftperr);
-
- closeServer(); // we failed, so no serverComplete()
+ if (!err) {
+ fwd->request->detailError(error, xerrno);
+ fwd->fail(ftperr);
+ closeServer(); // we failed, so no serverComplete()
+ }
}
Http::StatusCode
Ftp::Client::failedHttpStatus(err_type &error)
{
if (error == ERR_NONE)
error = ERR_FTP_FAILURE;
return error == ERR_READ_TIMEOUT ? Http::scGatewayTimeout :
Http::scBadGateway;
}
/**
* DPW 2007-04-23
* Looks like there are no longer anymore callers that set
* buffered_ok=1. Perhaps it can be removed at some point.
*/
void
Ftp::Client::scheduleReadControlReply(int buffered_ok)
{
debugs(9, 3, ctrl.conn);
=== modified file 'src/clients/FtpClient.h'
--- src/clients/FtpClient.h 2015-01-13 07:25:36 +0000
+++ src/clients/FtpClient.h 2015-10-11 14:52:02 +0000
@@ -81,41 +81,42 @@
void addr(const Ip::Address &addr); ///< import host and port
public:
MemBuf *readBuf;
char *host;
unsigned short port;
bool read_pending;
};
/// FTP client functionality shared among FTP Gateway and Relay clients.
class Client: public ::Client
{
CBDATA_CLASS(Client);
public:
explicit Client(FwdState *fwdState);
virtual ~Client();
/// handle a fatal transaction error, closing the control connection
- virtual void failed(err_type error = ERR_NONE, int xerrno = 0);
+ virtual void failed(err_type error = ERR_NONE, int xerrno = 0,
+ ErrorState *ftperr = NULL);
/// read timeout handler
virtual void timeout(const CommTimeoutCbParams &io);
/* Client API */
virtual void maybeReadVirginBody();
void writeCommand(const char *buf);
/// extracts remoteAddr from PASV response, validates it,
/// sets data address details, and returns true on success
bool handlePasvReply(Ip::Address &remoteAddr);
bool handleEpsvReply(Ip::Address &remoteAddr);
bool sendEprt();
bool sendPort();
bool sendPassive();
void connectDataChannel();
bool openListenSocket();
void switchTimeoutToDataChannel();
=== modified file 'src/clients/FtpGateway.cc'
--- src/clients/FtpGateway.cc 2015-08-24 16:51:17 +0000
+++ src/clients/FtpGateway.cc 2015-10-11 15:32:07 +0000
@@ -1210,86 +1210,62 @@
ftpSendUser(ftpState);
} else if (code == 120) {
if (NULL != ftpState->ctrl.message)
debugs(9, DBG_IMPORTANT, "FTP server is busy: " << ftpState->ctrl.message->key);
return;
} else {
ftpFail(ftpState);
}
}
/**
* Translate FTP login failure into HTTP error
* this is an attmpt to get the 407 message to show up outside Squid.
* its NOT a general failure. But a correct FTP response type.
*/
void
Ftp::Gateway::loginFailed()
{
ErrorState *err = NULL;
- const char *command, *reply;
if ((state == SENT_USER || state == SENT_PASS) && ctrl.replycode >= 400) {
if (ctrl.replycode == 421 || ctrl.replycode == 426) {
// 421/426 - Service Overload - retry permitted.
err = new ErrorState(ERR_FTP_UNAVAILABLE, Http::scServiceUnavailable, fwd->request);
} else if (ctrl.replycode >= 430 && ctrl.replycode <= 439) {
// 43x - Invalid or Credential Error - retry challenge required.
err = new ErrorState(ERR_FTP_FORBIDDEN, Http::scUnauthorized, fwd->request);
} else if (ctrl.replycode >= 530 && ctrl.replycode <= 539) {
// 53x - Credentials Missing - retry challenge required
if (password_url) // but they were in the URI! major fail.
err = new ErrorState(ERR_FTP_FORBIDDEN, Http::scForbidden, fwd->request);
else
err = new ErrorState(ERR_FTP_FORBIDDEN, Http::scUnauthorized, fwd->request);
}
}
- // any other problems are general falures.
- if (!err) {
- ftpFail(this);
+ if (!err)
return;
- }
-
- err->ftp.server_msg = ctrl.message;
-
- ctrl.message = NULL;
-
- if (old_request)
- command = old_request;
- else
- command = ctrl.last_command;
- if (command && strncmp(command, "PASS", 4) == 0)
- command = "PASS <yourpassword>";
-
- if (old_reply)
- reply = old_reply;
- else
- reply = ctrl.last_reply;
-
- if (command)
- err->ftp.request = xstrdup(command);
-
- if (reply)
- err->ftp.reply = xstrdup(reply);
+ failed(ERR_NONE, ctrl.replycode, err);
+ // any other problems are general falures.
HttpReply *newrep = err->BuildHttpReply();
delete err;
#if HAVE_AUTH_MODULE_BASIC
/* add Authenticate header */
newrep->header.putAuth("Basic", ftpRealm());
#endif
// add it to the store entry for response....
entry->replaceHttpReply(newrep);
serverComplete();
}
const char *
Ftp::Gateway::ftpRealm()
{
static char realm[8192];
/* This request is not fully authenticated */
@@ -2396,67 +2372,78 @@
debugs(9, 3, HERE);
if (old_request == NULL) {
old_request = ctrl.last_command;
ctrl.last_command = NULL;
old_reply = ctrl.last_reply;
ctrl.last_reply = NULL;
if (pathcomps == NULL && filepath != NULL)
old_filepath = xstrdup(filepath);
}
/* Jump to the "hack" state */
nextState(this);
}
static void
ftpFail(Ftp::Gateway *ftpState)
{
const bool slashHack = ftpState->request->url.path().caseCmp("/%2f", 4)==0;
- debugs(9, 6, "flags(" <<
+ int code = ftpState->ctrl.replycode;
+ err_type error_code = ERR_NONE;
+
+ debugs(9, 6, "state " << ftpState->state <<
+ " reply code " << code << "flags(" <<
(ftpState->flags.isdir?"IS_DIR,":"") <<
(ftpState->flags.try_slash_hack?"TRY_SLASH_HACK":"") << "), " <<
"mdtm=" << ftpState->mdtm << ", size=" << ftpState->theSize <<
"slashhack=" << (slashHack? "T":"F"));
/* Try the / hack to support "Netscape" FTP URL's for retreiving files */
if (!ftpState->flags.isdir && /* Not a directory */
!ftpState->flags.try_slash_hack && !slashHack && /* Not doing slash hack */
ftpState->mdtm <= 0 && ftpState->theSize < 0) { /* Not known as a file */
switch (ftpState->state) {
case Ftp::Client::SENT_CWD:
case Ftp::Client::SENT_RETR:
/* Try the / hack */
ftpState->hackShortcut(ftpTrySlashHack);
return;
default:
break;
}
}
- ftpState->failed(ERR_NONE, 0);
- /* failed() closes ctrl.conn and frees this */
+ Http::StatusCode sc = ftpState->failedHttpStatus(error_code);
+ ErrorState *ftperr = new ErrorState(error_code, sc, ftpState->fwd->request);
+ ftpState->failed(error_code, code, ftperr);
+ ftperr->detailError(code);
+ HttpReply *newrep = ftperr->BuildHttpReply();
+ delete ftperr;
+
+ ftpState->entry->replaceHttpReply(newrep);
+ ftpSendQuit(ftpState);
}
Http::StatusCode
Ftp::Gateway::failedHttpStatus(err_type &error)
{
if (error == ERR_NONE) {
switch (state) {
case SENT_USER:
case SENT_PASS:
if (ctrl.replycode > 500) {
error = ERR_FTP_FORBIDDEN;
return password_url ? Http::scForbidden : Http::scUnauthorized;
} else if (ctrl.replycode == 421) {
error = ERR_FTP_UNAVAILABLE;
return Http::scServiceUnavailable;
}
break;
More information about the squid-dev
mailing list