]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
ESI: remove custom parser (#128)
authorAmos Jeffries <yadij@users.noreply.github.com>
Thu, 18 Jan 2018 20:54:50 +0000 (09:54 +1300)
committerAmos Jeffries <yadij@users.noreply.github.com>
Fri, 19 Jan 2018 09:04:36 +0000 (22:04 +1300)
Alex Rousskov:
  let's consider removing the custom ESI parser from Squid. It is of
terrible quality and "nobody" is testing ESI code when things change. Is
the CVE risk worth supporting few platforms that do not have the right
parser libraries?

* Fixed configure.ac tests for ESI libraries

configure.ac
src/cf.data.pre
src/esi/CustomParser.cc [deleted file]
src/esi/CustomParser.h [deleted file]
src/esi/Makefile.am
src/esi/Module.cc
src/esi/Parser.cc
test-suite/buildtests/layer-05-nodeps-esi.opts [deleted file]

index 1ba8984420f41ed27268df7d5d9f7272d9dab0ea..cb0e99d3bf497804d9eae9acbcfe26eddda16d9c 100644 (file)
@@ -890,40 +890,38 @@ AC_ARG_ENABLE(delay-pools,
 dnl disable generic/common adaptation support by default
 squid_opt_use_adaptation=no
 
-squid_opt_use_esi=yes
-AH_TEMPLATE([USE_SQUID_ESI],
-            [Define to enable the ESI processor and Surrogate header support])
+squid_opt_use_esi=auto
+AH_TEMPLATE([USE_SQUID_ESI],[Define to enable the ESI processor])
 AC_ARG_ENABLE(esi,
-  AS_HELP_STRING([--enable-esi],
-                 [Enable ESI for accelerators. Benefits from expat or libxml2.
+  AS_HELP_STRING([--disable-esi],
+                 [Disable ESI for accelerators. ESI requires expat or libxml2.
                   Enabling ESI will cause squid reverse proxies to be capable
                   of the Edge Acceleration Specification (www.esi.org).]),
-                 [squid_opt_use_esi=$enableval], [squid_opt_use_esi=no])
+                 [squid_opt_use_esi=$enableval],[])
 HAVE_LIBEXPAT=0
 EXPATLIB=
 HAVE_LIBXML2=0
 XMLLIB=
-if test "x$squid_opt_use_esi" = "xyes" ; then
-  AC_MSG_NOTICE([Enabling ESI processor and Surrogate header support.])
-  AC_DEFINE(USE_SQUID_ESI,1,
-    [Compile the ESI processor and Surrogate header support])
-else
-  AC_MSG_NOTICE([Disabling ESI processor])
-fi
 
 # ESI support libraries: expat
 AC_ARG_WITH(expat, AS_HELP_STRING([--without-expat],[Do not use expat for ESI. Default: auto-detect]))
-if test "x$squid_opt_use_esi" = "xyes" -a "x$with_expat" != "xno" ; then
+if test "x$squid_opt_use_esi" != "xno" -a "x$with_expat" != "xno" ; then
   AC_CHECK_LIB([expat], [main], [EXPATLIB="-lexpat"; HAVE_LIBEXPAT=1])
   AC_CHECK_HEADERS([expat.h])
   AC_DEFINE_UNQUOTED(HAVE_LIBEXPAT, $HAVE_LIBEXPAT, [Define to 1 if you have the expat library])
-  if test "x$with_expat" = "xyes" -a "x$HAVE_LIBEXPAT" != "x1" ; then
-    AC_MSG_ERROR([Required library expat is not able to be found.])
-  fi
+  AS_IF(test "x$HAVE_LIBEXPAT" = "x1",[
+    squid_opt_use_esi=yes
+  ],[
+    AS_IF(test "x$with_expat" = "xyes",[
+      AC_MSG_ERROR([Required library expat not found.])
+    ],[
+      AC_MSG_NOTICE([Library expat not found.])
+    ])
+  ])
 fi
 
 AC_ARG_WITH(libxml2, AS_HELP_STRING([--without-libxml2],[Do not use libxml2 for ESI. Default: auto-detect]))
-if test "x$squid_opt_use_esi" = "xyes" -a "x$with_libxml2" != "xno" ; then
+if test "x$squid_opt_use_esi" != "xno" -a "x$with_libxml2" != "xno" ; then
   AC_CHECK_LIB([xml2], [main], [XMLLIB="-lxml2"; HAVE_LIBXML2=1])
   dnl Find the main header and include path...
   AC_CACHE_CHECK([location of libxml2 include files], [ac_cv_libxml2_include], [
@@ -950,15 +948,34 @@ if test "x$squid_opt_use_esi" = "xyes" -a "x$with_libxml2" != "xno" ; then
   dnl Now that we know where to look find the headers...
   AC_CHECK_HEADERS(libxml/parser.h libxml/HTMLparser.h libxml/HTMLtree.h)
   AC_DEFINE_UNQUOTED(HAVE_LIBXML2, $HAVE_LIBXML2, [Define to 1 if you have the libxml2 library])
-  if test "x$with_libxml2" = "xyes" -a "$HAVE_LIBXML2" != "1" ; then
-    AC_MSG_ERROR([Required library libxml2 is not able to be found.])
-  fi
+  AS_IF(test "x$HAVE_LIBXML2" = "x1",[
+    squid_opt_use_esi=yes
+  ],[
+    AS_IF(test "x$with_libxml2" = "xyes",[
+      AC_MSG_ERROR([Required library libxml2 not found.])
+    ],[
+      AC_MSG_NOTICE([Library libxml2 not found.])
+    ])
+  ])
 fi
 
+AS_IF([test "x$squid_opt_use_esi" = "xyes"],[
+  AS_IF(test "x$HAVE_LIBXML2" = "x0" -a "x$HAVE_LIBEXPAT" = "x0",[
+    AC_MSG_ERROR([ESI processor requires libxml2 or libexpat])
+  ])
+  AC_MSG_NOTICE([Enabling ESI processor: $EXPATLIB $XMLLIB])
+  AC_DEFINE(USE_SQUID_ESI,1,[Compile the ESI processor])
+],[
+  AS_IF(test "x$squid_opt_use_esi" = "xno",[
+    AC_MSG_NOTICE([Disabling ESI processor.])
+  ],[
+    AC_MSG_NOTICE([Disabling ESI processor. libxml2 and libexpat not found.])
+  ])
+])
 AM_CONDITIONAL(ENABLE_ESI, test "x$squid_opt_use_esi" = "xyes")
-AM_CONDITIONAL(ENABLE_LIBEXPAT, test "$HAVE_LIBEXPAT" = 1)
+AM_CONDITIONAL(ENABLE_LIBEXPAT, test "x$HAVE_LIBEXPAT" = "x1")
 AC_SUBST(EXPATLIB)
-AM_CONDITIONAL(ENABLE_LIBXML2, test "$HAVE_LIBXML2" = 1)
+AM_CONDITIONAL(ENABLE_LIBXML2, test "x$HAVE_LIBXML2" = "x1")
 AC_SUBST(XMLLIB)
 
 # icap argument handling
index fef145ad2d694b7333658c29cee1169a7e17c196..787ff974e32f403718e7ab186690fc2e106bcaf0 100644 (file)
@@ -6906,14 +6906,16 @@ DOC_END
 
 NAME: esi_parser
 IFDEF: USE_SQUID_ESI
-COMMENT: libxml2|expat|custom
+COMMENT: libxml2|expat
 TYPE: string
 LOC: ESIParser::Type
-DEFAULT: custom
+DEFAULT: auto
+DEFAULT_DOC: Selects libxml2 if available at ./configure time or libexpat otherwise.
 DOC_START
-       ESI markup is not strictly XML compatible. The custom ESI parser
-       will give higher performance, but cannot handle non ASCII character
-       encodings.
+       Selects the XML parsing library to use when interpreting responses with
+       Edge Side Includes.
+
+       To disable ESI handling completely, ./configure Squid with --disable-esi.
 DOC_END
 
 COMMENT_START
diff --git a/src/esi/CustomParser.cc b/src/esi/CustomParser.cc
deleted file mode 100644 (file)
index 27d1c3b..0000000
+++ /dev/null
@@ -1,300 +0,0 @@
-/*
- * Copyright (C) 1996-2018 The Squid Software Foundation and contributors
- *
- * Squid software is distributed under GPLv2+ license and includes
- * contributions from numerous individuals and organizations.
- * Please see the COPYING and CONTRIBUTORS files for details.
- */
-
-/* DEBUG: section 86    ESI processing */
-
-#include "squid.h"
-#include "Debug.h"
-#include "esi/CustomParser.h"
-#include "fatal.h"
-#include "libTrie/Trie.h"
-#include "libTrie/TrieCharTransform.h"
-
-#include <vector>
-
-Trie *ESICustomParser::SearchTrie=NULL;
-
-EsiParserDefinition(ESICustomParser);
-
-Trie *
-ESICustomParser::GetTrie()
-{
-    if (SearchTrie)
-        return SearchTrie;
-
-    SearchTrie = new Trie(new TrieCaseless);
-
-    static const ESITAG_t ESITAG_value = ESITAG;
-
-    assert (SearchTrie->add
-            ("<esi:",5,(void *)&ESITAG_value));
-
-    static const ESITAG_t ESIENDTAG_value = ESIENDTAG;
-
-    assert (SearchTrie->add
-            ("</esi:",6,(void *)&ESIENDTAG_value));
-
-    static const ESITAG_t ESICOMMENT_value = ESICOMMENT;
-
-    assert (SearchTrie->add
-            ("<!--",4,(void *)&ESICOMMENT_value));
-
-    return SearchTrie;
-}
-
-ESICustomParser::ESICustomParser(ESIParserClient *aClient) :
-    theClient(aClient),
-    lastTag(ESITAG)
-{}
-
-ESICustomParser::~ESICustomParser()
-{
-    theClient = NULL;
-}
-
-char const *
-ESICustomParser::findTag(char const *buffer, size_t bufferLength)
-{
-    size_t myOffset (0);
-    ESITAG_t *resulttype = NULL;
-
-    while (myOffset < bufferLength &&
-            (resulttype = static_cast<ESITAG_t *>(GetTrie()->findPrefix (buffer + myOffset, bufferLength - myOffset)))
-            == NULL)
-        ++myOffset;
-
-    if (myOffset == bufferLength)
-        return NULL;
-
-    debugs(86, 9, "ESICustomParser::findTag: found " << *resulttype);
-
-    lastTag = *resulttype;
-
-    return buffer + myOffset;
-}
-
-bool
-ESICustomParser::parse(char const *dataToParse, size_t const lengthOfData, bool const endOfStream)
-{
-    debugs(86, 9, "ESICustomParser::parse: Appending data to internal buffer");
-    content.append (dataToParse, lengthOfData);
-
-    if (!endOfStream) {
-        return true;
-    }
-
-    size_t openESITags (0);
-    // TODO: convert to Tokenizer parse
-    // erring on the safe side for now. Probably rawContent would be ok too
-    // note that operations below do *X='\0' ... altering the 'const' buffer content.
-    char const *currentPos = content.c_str();
-    SBuf::size_type remainingCount = content.length();
-    char const *tag = NULL;
-
-    while ((tag = findTag(currentPos, remainingCount))) {
-        if (tag - currentPos)
-            theClient->parserDefault (currentPos,tag - currentPos);
-
-        switch (lastTag) {
-
-        case ESITAG: {
-            ++openESITags;
-            char *tagEnd = strchr(const_cast<char *>(tag), '>');
-
-            if (!tagEnd) {
-                error = "Could not find end ('>') of tag";
-                return false;
-            }
-
-            if (tagEnd - tag > (ssize_t)remainingCount) {
-                error = "Tag ends beyond the parse buffer.";
-                return false;
-            }
-
-            if (*(tagEnd - 1) == '/')
-                --openESITags;
-
-            char * endofName = strpbrk(const_cast<char *>(tag), w_space);
-
-            if (endofName > tagEnd)
-                endofName = const_cast<char *>(tagEnd);
-
-            *endofName = '\0';
-
-            *tagEnd = '\0';
-
-            std::vector<char *>attributes;
-
-            char *attribute = const_cast<char *>(endofName + 1);
-
-            while (attribute > tag && attribute < tagEnd) {
-                /* leading spaces */
-
-                while (attribute < tagEnd && (xisspace(*attribute) || (*attribute == '/')))
-                    ++attribute;
-
-                if (! (attribute < tagEnd))
-                    break;
-
-                /* attribute name */
-                attributes.push_back(attribute);
-
-                char *nextSpace = strpbrk(attribute, w_space);
-
-                char *equals = strchr(attribute, '=');
-
-                if (!equals) {
-                    error = "Missing attribute value.";
-                    return false;
-                }
-
-                if (nextSpace && nextSpace < equals)
-                    *nextSpace = '\0';
-                else
-                    *equals = '\0';
-
-                ++equals;
-
-                while (equals < tagEnd && xisspace(*equals))
-                    ++equals;
-
-                char sep = *equals;
-
-                if (sep != '\'' && sep != '"') {
-                    error = "Unknown identifier (";
-                    error.append (sep);
-                    error.append (")");
-                    return false;
-                }
-
-                char *value = equals + 1;
-                char *end = strchr(value, sep);
-
-                if (!end) {
-                    error = "Missing attribute ending separator (";
-                    error.append(sep);
-                    error.append(")");
-                    return false;
-                }
-                attributes.push_back(value);
-                *end = '\0';
-                attribute = end + 1;
-            }
-
-            // TODO: after c++11, replace &attributes.front() with attributes.data()
-            theClient->start (tag + 1, const_cast<const char **>(&attributes.front()), attributes.size() >> 1);
-            /* TODO: attributes */
-
-            if (*(tagEnd - 1) == '/')
-                theClient->end (tag + 1);
-
-            remainingCount -= tagEnd - currentPos + 1;
-
-            currentPos = tagEnd + 1;
-        }
-
-        break;
-
-        case ESIENDTAG: {
-            if (!openESITags)
-                return false;
-
-            char const *tagEnd = strchr(tag, '>');
-
-            if (!tagEnd)
-                return false;
-
-            if (tagEnd - tag > (ssize_t)remainingCount)
-                return false;
-
-            char * endofName = strpbrk(const_cast<char *>(tag), w_space);
-
-            if (endofName > tagEnd)
-                endofName = const_cast<char *>(tagEnd);
-
-            *endofName = '\0';
-
-            theClient->end (tag + 2);
-
-            --openESITags;
-
-            remainingCount -= tagEnd - currentPos + 1;
-
-            currentPos = tagEnd + 1;
-        }
-
-        break;
-
-        case ESICOMMENT: {
-            /* Further optimisation potential:
-             * 1) recognize end comments for esi and don't callback on
-             * comments.
-             * 2) provide the comment length to the caller.
-             */
-            /* Comments must not be nested, without CDATA
-             * and we don't support CDATA
-             */
-            char *commentEnd = strstr (const_cast<char *>(tag), "-->");
-
-            if (!commentEnd) {
-                error = "missing end of comment";
-                return false;
-            }
-
-            if (commentEnd - tag > (ssize_t)remainingCount) {
-                error = "comment ends beyond parse buffer";
-                return false;
-            }
-
-            *commentEnd = '\0';
-            theClient->parserComment (tag + 4);
-            remainingCount -= commentEnd - currentPos + 3;
-            currentPos = commentEnd + 3;
-        }
-
-        break;
-        break;
-
-        default:
-            fatal ("unknown ESI tag type found");
-        };
-
-        /*
-         * Find next esi tag (open or closing) or comment
-         * send tag, or full comment text
-         * rinse
-         */
-    }
-
-    if (remainingCount)
-        theClient->parserDefault (currentPos,remainingCount);
-
-    debugs(86, 5, "ESICustomParser::parse: Finished parsing, will return " << !openESITags);
-
-    if (openESITags)
-        error = "ESI Tags still open";
-
-    return !openESITags;
-}
-
-long int
-ESICustomParser::lineNumber() const
-{
-    /* We don't track lines in the body */
-    return 0;
-}
-
-char const *
-ESICustomParser::errorString() const
-{
-    if (error.size())
-        return error.termedBuf();
-    else
-        return "Parsing error strings not implemented";
-}
-
diff --git a/src/esi/CustomParser.h b/src/esi/CustomParser.h
deleted file mode 100644 (file)
index 5430513..0000000
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
- * Copyright (C) 1996-2018 The Squid Software Foundation and contributors
- *
- * Squid software is distributed under GPLv2+ license and includes
- * contributions from numerous individuals and organizations.
- * Please see the COPYING and CONTRIBUTORS files for details.
- */
-
-#ifndef SQUID_ESICUSTOMPARSER_H
-#define SQUID_ESICUSTOMPARSER_H
-
-class Trie;
-
-/* inherits from */
-#include "esi/Parser.h"
-
-#include "sbuf/SBuf.h"
-#include "SquidString.h"
-
-/**
- \ingroup ESIAPI
- */
-class ESICustomParser : public ESIParser
-{
-
-public:
-    ESICustomParser(ESIParserClient *);
-    ~ESICustomParser();
-    /* true on success */
-    bool parse(char const *dataToParse, size_t const lengthOfData, bool const endOfStream);
-    long int lineNumber() const;
-    char const * errorString() const;
-
-    EsiParserDeclaration;
-
-private:
-    static Trie *SearchTrie;
-    static Trie *GetTrie();
-    enum ESITAG_t {
-        ESITAG=1,
-        ESIENDTAG=2,
-        ESICOMMENT=3
-    };
-
-    char const *findTag(char const *a, size_t b);
-    ESIParserClient *theClient;
-    String error;
-    /* cheap n dirty - buffer it all */
-    SBuf content;
-    /* TODO: make a class of this type code */
-    ESITAG_t lastTag;
-};
-
-#endif /* SQUID_ESICUSTOMPARSER_H */
-
index 02b4947f054fd14d72ab096a62d3145e9b4a119b..49c237f4deeb3785a48e63aafd2c3d97a71bd617 100644 (file)
@@ -10,9 +10,7 @@ include $(top_srcdir)/src/TestHeaders.am
 
 noinst_LTLIBRARIES = libesi.la
 
-ESI_PARSER_SOURCES = \
-       CustomParser.cc \
-       CustomParser.h
+ESI_PARSER_SOURCES =
 
 if ENABLE_LIBEXPAT
 ESI_PARSER_SOURCES += \
index 2a163be7f80b7e544022ce101de8cf10d5419381..bced5a8f6ba44c74190bbadb8091700f393df3f2 100644 (file)
@@ -7,14 +7,12 @@
  */
 
 #include "squid.h"
-#include "esi/CustomParser.h"
 #include "esi/Libxml2Parser.h"
 #include "esi/Module.h"
 /* include for esi/ExpatParser.h must follow esi/Libxml2Parser.h */
 /* do not remove this comment, as it acts as barrier for the autmatic sorting */
 #include "esi/ExpatParser.h"
 
-static ESIParser::Register *prCustom = 0;
 #if HAVE_LIBXML2
 static ESIParser::Register *prLibxml = 0;
 #endif
@@ -24,23 +22,21 @@ static ESIParser::Register *prExpat = 0;
 
 void Esi::Init()
 {
-    assert(!prCustom); // we should be called once
-
-    prCustom = new ESIParser::Register("custom", &ESICustomParser::NewParser);
+    // register in reverse order of preference.
+    // The latest registered parser will be used as default.
+#if HAVE_LIBEXPAT
+    assert(!prExpat); // we should be called once
+    prExpat = new ESIParser::Register("expat", &ESIExpatParser::NewParser);
+#endif
 
 #if HAVE_LIBXML2
+    assert(!prLibxml); // we should be called once
     prLibxml = new ESIParser::Register("libxml2", &ESILibxml2Parser::NewParser);
 #endif
-
-#if HAVE_LIBEXPAT
-    prExpat = new ESIParser::Register("expat", &ESIExpatParser::NewParser);
-#endif
 }
 
 void Esi::Clean()
 {
-    assert(prCustom); // we should be called once, and only after Init()
-
 #if HAVE_LIBEXPAT
     delete prExpat;
     prExpat = NULL;
@@ -50,8 +46,5 @@ void Esi::Clean()
     delete prLibxml;
     prLibxml = NULL;
 #endif
-
-    delete prCustom;
-    prCustom = NULL;
 }
 
index 393eea260241d3d9053bc9475aac4005ffcad894..4d81ef59c60255d36d92d7b1dba56d5247899b3c 100644 (file)
@@ -22,8 +22,11 @@ ESIParser::NewParser(ESIParserClient *aClient)
     if (Parser == NULL) {
         Parser = Parsers;
 
-        while (Parser != NULL && strcasecmp(Parser->name, Type) != 0)
-            Parser = Parser->next;
+        // if type name matters, use it
+        if (strcasecmp(Type, "auto") != 0) {
+            while (Parser && strcasecmp(Parser->name, Type) != 0)
+                Parser = Parser->next;
+        }
 
         if (Parser == NULL)
             fatal ("Unknown ESI Parser type");
diff --git a/test-suite/buildtests/layer-05-nodeps-esi.opts b/test-suite/buildtests/layer-05-nodeps-esi.opts
deleted file mode 100644 (file)
index 045bf33..0000000
+++ /dev/null
@@ -1,29 +0,0 @@
-## Copyright (C) 1996-2018 The Squid Software Foundation and contributors
-##
-## Squid software is distributed under GPLv2+ license and includes
-## contributions from numerous individuals and organizations.
-## Please see the COPYING and CONTRIBUTORS files for details.
-##
-
-#
-# Component Missing Libraries Check - everything MUST work at this level
-MAKETEST="distcheck"
-#
-# Currently tested components with plug-in-play capability:
-#
-#      ESI - optional libexpat, libxml2
-#
-# NP: there must be no overlap in code for plugging the libraries in/out.
-#     this means we can test the absence of all in one run and save time.
-#
-# NP: DISTCHECK_CONFIGURE_FLAGS is a magic automake macro for the 
-#     distcheck target recursive tests beteen scripted runs.
-#     we use it to perform the same duty between our nested scripts.
-DISTCHECK_CONFIGURE_FLAGS="\
-       --enable-esi \
-       --without-expat \
-       --without-libxml2 \
-       --enable-build-info"
-
-# Fix the distclean testing.
-export DISTCHECK_CONFIGURE_FLAGS