Re: /bzr/squid3/trunk/ r9386: Bug 2395: FTP auth errors not displayed

Isn't the real question who called FwdState::complete()? It's only meant
to be called when all data has been placed into the entry, not before..

tis 2008-12-02 klockan 00:56 +1300 skrev Amos Jeffries:
> ------------------------------------------------------------
> revno: 9386
> committer: Amos Jeffries <squid3_at_treenet.co.nz>
> branch nick: trunk
> timestamp: Tue 2008-12-02 00:56:34 +1300
> message:
> Bug 2395: FTP auth errors not displayed
>
> I appears to be the StoreEntry reporting an error on zero-length objects. This
> somehow overrides the FTP reported error and aborts the reply page.
>
> Add an extra check to prevent StoreEntry::complete() being called too
> early on error responses.
> modified:
> src/forward.cc
> src/ftp.cc
> vanligt textdokument-bilaga (r9386.diff)
> === modified file 'src/forward.cc'
> --- a/src/forward.cc 2008-10-16 04:51:12 +0000
> +++ b/src/forward.cc 2008-12-01 11:56:34 +0000
> @@ -335,9 +335,12 @@
>
> startComplete(servers);
> } else {
> - debugs(17, 3, "fwdComplete: not re-forwarding status " << entry->getReply()->sline.status);
> - EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
> - entry->complete();
> + debugs(17, 3, "fwdComplete: server FD " << server_fd << " not re-forwarding status " << entry->getReply()->sline.status);
> + if (entry->isEmpty() && !err)
> + {
> + EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
> + entry->complete();
> + }
>
> if (server_fd < 0)
> completed();
>
> === modified file 'src/ftp.cc'
> --- a/src/ftp.cc 2008-09-24 13:21:04 +0000
> +++ b/src/ftp.cc 2008-12-01 11:56:34 +0000
> @@ -1991,7 +1991,7 @@
> ftpReadPass(FtpStateData * ftpState)
> {
> int code = ftpState->ctrl.replycode;
> - debugs(9, 3, HERE);
> + debugs(9, 3, HERE << "code=" << code);
>
> if (code == 230) {
> ftpSendType(ftpState);
> @@ -3462,7 +3462,11 @@
> static void
> ftpFail(FtpStateData *ftpState)
> {
> - debugs(9, 3, HERE);
> + debugs(9, 6, HERE << "flags(" <<
> + (ftpState->flags.isdir?"IS_DIR,":"") <<
> + (ftpState->flags.try_slash_hack?"TRY_SLASH_HACK":"") << "), " <<
> + "mdtm=" << ftpState->mdtm << ", size=" << ftpState->theSize <<
> + "slashhack=" << (ftpState->request->urlpath.caseCmp("/%2f", 4)==0? "T":"F") );
>
> /* Try the / hack to support "Netscape" FTP URL's for retreiving files */
> if (!ftpState->flags.isdir && /* Not a directory */
> @@ -3491,6 +3495,7 @@
> void
> FtpStateData::failed(err_type error, int xerrno)
> {
> + debugs(9,3,HERE << "entry-null=" << (entry?entry->isEmpty():0) << ", entry=" << entry);
> if (entry->isEmpty())
> failedErrorMessage(error, xerrno);
>
>