------------------------------------------------------------
revno: 13671
revision-id: squid3@treenet.co.nz-20141203114356-3ah5egxm478w0rdv
parent: squid3@treenet.co.nz-20141203114128-i9f1ou00f6q97k09
author: Christos Tsantilas <chtsanti@users.sourceforge.net>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Wed 2014-12-03 03:43:56 -0800
message:
  FTP FEAT error handling
  
  Some FTP severs respond to a FEAT command with 5xx status code. Squid sends 
  an invalid response in these cases which can confuse the client.
  
  This patch fixes Squid to always send a valid 211 reply to client which
  lists at least the EPSV and EPRT ftp commands which supported by Squid
  regardless of the origin server support.
  
  This patch also fixes a memory leak when FEAT replies processed.
  
  This is a Measurement Factory project
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20141203114356-3ah5egxm478w0rdv
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: c6b16fa4cf9bb85c1f382cd2ae4c36eb4126e788
# timestamp: 2014-12-03 11:45:04 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20141203114128-\
#   i9f1ou00f6q97k09
# 
# Begin patch
=== modified file 'src/servers/FtpServer.cc'
--- src/servers/FtpServer.cc	2014-10-31 01:43:16 +0000
+++ src/servers/FtpServer.cc	2014-12-03 11:43:56 +0000
@@ -800,66 +800,64 @@
         return;
     }
 
-    HttpReply *filteredReply = reply->clone();
-    HttpHeader &filteredHeader = filteredReply->header;
+    HttpReply::Pointer featReply = Ftp::HttpReplyWrapper(211, "End", Http::scNoContent, 0);
+    HttpHeader const &serverReplyHeader = reply->header;
 
-    // Remove all unsupported commands from the response wrapper.
-    int deletedCount = 0;
     HttpHeaderPos pos = HttpHeaderInitPos;
     bool hasEPRT = false;
     bool hasEPSV = false;
     int prependSpaces = 1;
-    while (const HttpHeaderEntry *e = filteredHeader.getEntry(&pos)) {
-        if (e->id == HDR_FTP_PRE) {
-            // assume RFC 2389 FEAT response format, quoted by Squid:
-            // <"> SP NAME [SP PARAMS] <">
-            // but accommodate MS servers sending four SPs before NAME
-
-            // command name ends with (SP parameter) or quote
-            static const CharacterSet AfterFeatNameChars("AfterFeatName", " \"");
-            static const CharacterSet FeatNameChars = AfterFeatNameChars.complement("FeatName");
-
-            Parser::Tokenizer tok(SBuf(e->value.termedBuf()));
-            if (!tok.skip('"') && !tok.skip(' '))
-                continue;
-
-            // optional spaces; remember their number to accomodate MS servers
-            prependSpaces = 1 + tok.skipAll(CharacterSet::SP);
-
-            SBuf cmd;
-            if (!tok.prefix(cmd, FeatNameChars))
-                continue;
-            cmd.toUpper();
-
-            if (!Ftp::SupportedCommand(cmd))
-                filteredHeader.delAt(pos, deletedCount);
-
-            if (cmd == cmdEprt())
-                hasEPRT = true;
-            else if (cmd == cmdEpsv())
-                hasEPSV = true;
+
+    featReply->header.putStr(HDR_FTP_PRE, "\"211-Features:\"");
+    const int scode = serverReplyHeader.getInt(HDR_FTP_STATUS);
+    if (scode == 211) {
+        while (const HttpHeaderEntry *e = serverReplyHeader.getEntry(&pos)) {
+            if (e->id == HDR_FTP_PRE) {
+                // assume RFC 2389 FEAT response format, quoted by Squid:
+                // <"> SP NAME [SP PARAMS] <">
+                // but accommodate MS servers sending four SPs before NAME
+
+                // command name ends with (SP parameter) or quote
+                static const CharacterSet AfterFeatNameChars("AfterFeatName", " \"");
+                static const CharacterSet FeatNameChars = AfterFeatNameChars.complement("FeatName");
+
+                Parser::Tokenizer tok(SBuf(e->value.termedBuf()));
+                if (!tok.skip('"') || !tok.skip(' '))
+                    continue;
+
+                // optional spaces; remember their number to accomodate MS servers
+                prependSpaces = 1 + tok.skipAll(CharacterSet::SP);
+
+                SBuf cmd;
+                if (!tok.prefix(cmd, FeatNameChars))
+                    continue;
+                cmd.toUpper();
+
+                if (Ftp::SupportedCommand(cmd)) {
+                    featReply->header.addEntry(e->clone());
+                }
+
+                if (cmd == cmdEprt())
+                    hasEPRT = true;
+                else if (cmd == cmdEpsv())
+                    hasEPSV = true;
+            }
         }
-    }
+    } // else we got a FEAT error and will only report Squid-supported features
 
     char buf[256];
-    int insertedCount = 0;
     if (!hasEPRT) {
         snprintf(buf, sizeof(buf), "\"%*s\"", prependSpaces + 4, "EPRT");
-        filteredHeader.putStr(HDR_FTP_PRE, buf);
-        ++insertedCount;
+        featReply->header.putStr(HDR_FTP_PRE, buf);
     }
     if (!hasEPSV) {
         snprintf(buf, sizeof(buf), "\"%*s\"", prependSpaces + 4, "EPSV");
-        filteredHeader.putStr(HDR_FTP_PRE, buf);
-        ++insertedCount;
-    }
-
-    if (deletedCount || insertedCount) {
-        filteredHeader.refreshMask();
-        debugs(33, 5, "deleted " << deletedCount << " inserted " << insertedCount);
-    }
-
-    writeForwardedReply(filteredReply);
+        featReply->header.putStr(HDR_FTP_PRE, buf);
+    }
+
+    featReply->header.refreshMask();
+
+    writeForwardedReply(featReply.getRaw());
 }
 
 void