diff --git a/src/esi/Context.h b/src/esi/Context.h index be49742..34b1fd0 100644 --- a/src/esi/Context.h +++ b/src/esi/Context.h @@ -12,6 +12,7 @@ #include "clientStream.h" #include "err_type.h" #include "esi/Element.h" +#include "esi/Esi.h" #include "esi/Parser.h" #include "http/StatusCode.h" #include "HttpReply.h" @@ -112,7 +113,7 @@ public: { public: - ESIElement::Pointer stack[10]; /* a stack of esi elements that are open */ + ESIElement::Pointer stack[ESI_STACK_DEPTH_LIMIT]; /* a stack of esi elements that are open */ int stackdepth; /* self explanatory */ ESIParser::Pointer theParser; ESIElement::Pointer top(); diff --git a/src/esi/Esi.cc b/src/esi/Esi.cc index 1816c76..674bae2 100644 --- a/src/esi/Esi.cc +++ b/src/esi/Esi.cc @@ -29,6 +29,7 @@ #include "esi/Expression.h" #include "esi/Segment.h" #include "esi/VarState.h" +#include "FadingCounter.h" #include "HttpHdrSc.h" #include "HttpHdrScTarget.h" #include "HttpReply.h" @@ -943,13 +944,18 @@ void ESIContext::addStackElement (ESIElement::Pointer element) { /* Put on the stack to allow skipping of 'invalid' markup */ - assert (parserState.stackdepth <11); + + // throw an error if the stack location would be invalid + if (parserState.stackdepth >= ESI_STACK_DEPTH_LIMIT) + throw Esi::Error("ESI Too many nested elements"); + if (parserState.stackdepth < 0) + throw Esi::Error("ESI elements stack error, probable error in ESI template"); + assert (!failed()); debugs(86, 5, "ESIContext::addStackElement: About to add ESI Node " << element.getRaw()); if (!parserState.top()->addElement(element)) { - debugs(86, DBG_IMPORTANT, "ESIContext::addStackElement: failed to add esi node, probable error in ESI template"); - flags.error = 1; + throw Esi::Error("ESIContext::addStackElement failed, probable error in ESI template"); } else { /* added ok, push onto the stack */ parserState.stack[parserState.stackdepth] = element; @@ -1201,13 +1207,10 @@ ESIContext::addLiteral (const char *s, int len) assert (len); debugs(86, 5, "literal length is " << len); /* give a literal to the current element */ - assert (parserState.stackdepth <11); ESIElement::Pointer element (new esiLiteral (this, s, len)); - if (!parserState.top()->addElement(element)) { - debugs(86, DBG_IMPORTANT, "ESIContext::addLiteral: failed to add esi node, probable error in ESI template"); - flags.error = 1; - } + if (!parserState.top()->addElement(element)) + throw Esi::Error("ESIContext::addLiteral failed, probable error in ESI template"); } void @@ -1269,8 +1272,24 @@ ESIContext::parse() PROF_start(esiParsing); - while (buffered.getRaw() && !flags.error) - parseOneBuffer(); + try { + while (buffered.getRaw() && !flags.error) + parseOneBuffer(); + + } catch (Esi::ErrorDetail &errMsg) { // FIXME: non-const for c_str() + // level-2: these are protocol/syntax errors from upstream + debugs(86, 2, "WARNING: ESI syntax error: " << errMsg); + setError(); + setErrorMessage(errMsg.c_str()); + + } catch (...) { + // DBG_IMPORTANT because these are local issues the admin needs to fix + static FadingCounter logEntries; // TODO: set horizon less than infinity + if (logEntries.count(1) < 100) + debugs(86, DBG_IMPORTANT, "ERROR: ESI parser: unhandled exception"); + setError(); + setErrorMessage("ESI parser error"); + } PROF_stop(esiParsing); diff --git a/src/esi/Esi.h b/src/esi/Esi.h index bbdb566..85f80f7 100644 --- a/src/esi/Esi.h +++ b/src/esi/Esi.h @@ -10,6 +10,11 @@ #define SQUID_ESI_H #include "clientStream.h" +#include "SBuf.h" + +#if !defined(ESI_STACK_DEPTH_LIMIT) +#define ESI_STACK_DEPTH_LIMIT 20 +#endif /* ESI.c */ extern CSR esiStreamRead; @@ -18,5 +23,14 @@ extern CSD esiStreamDetach; extern CSS esiStreamStatus; int esiEnableProcessing (HttpReply *); +namespace Esi +{ + +typedef SBuf ErrorDetail; +/// prepare an Esi::ErrorDetail for throw on ESI parser internal errors +inline Esi::ErrorDetail Error(const char *msg) { return ErrorDetail(msg); } + +} // namespace Esi + #endif /* SQUID_ESI_H */ diff --git a/src/esi/Expression.cc b/src/esi/Expression.cc index 8a1d3e9..a65edfb 100644 --- a/src/esi/Expression.cc +++ b/src/esi/Expression.cc @@ -10,6 +10,7 @@ #include "squid.h" #include "Debug.h" +#include "esi/Esi.h" #include "esi/Expression.h" #include "profiler/Profiler.h" @@ -97,6 +98,17 @@ stackpop(stackmember * s, int *depth) cleanmember(&s[*depth]); } +static void +stackpush(stackmember *stack, stackmember &item, int *depth) +{ + if (*depth < 0) + throw Esi::Error("ESIExpression stack has negative size"); + if (*depth >= ESI_STACK_DEPTH_LIMIT) + throw Esi::Error("ESIExpression stack is full, cannot push"); + + stack[(*depth)++] = item; +} + static evaluate evalnegate; static evaluate evalliteral; static evaluate evalor; @@ -208,6 +220,11 @@ evalnegate(stackmember * stack, int *depth, int whereAmI, stackmember * candidat /* invalid stack */ return 1; + if (whereAmI < 0) + throw Esi::Error("negate expression location too small"); + if (*depth >= ESI_STACK_DEPTH_LIMIT) + throw Esi::Error("negate expression too complex"); + if (stack[whereAmI + 1].valuetype != ESI_EXPR_EXPR) /* invalid operand */ return 1; @@ -280,7 +297,7 @@ evalor(stackmember * stack, int *depth, int whereAmI, stackmember * candidate) srv.precedence = 1; - stack[(*depth)++] = srv; + stackpush(stack, srv, depth); /* we're out of way, try adding now */ if (!addmember(stack, depth, candidate)) @@ -327,7 +344,7 @@ evaland(stackmember * stack, int *depth, int whereAmI, stackmember * candidate) srv.precedence = 1; - stack[(*depth)++] = srv; + stackpush(stack, srv, depth); /* we're out of way, try adding now */ if (!addmember(stack, depth, candidate)) @@ -373,7 +390,7 @@ evallesseq(stackmember * stack, int *depth, int whereAmI, stackmember * candidat srv.precedence = 1; - stack[(*depth)++] = srv; + stackpush(stack, srv, depth); /* we're out of way, try adding now */ if (!addmember(stack, depth, candidate)) @@ -421,7 +438,7 @@ evallessthan(stackmember * stack, int *depth, int whereAmI, stackmember * candid srv.precedence = 1; - stack[(*depth)++] = srv; + stackpush(stack, srv, depth); /* we're out of way, try adding now */ if (!addmember(stack, depth, candidate)) @@ -469,7 +486,7 @@ evalmoreeq(stackmember * stack, int *depth, int whereAmI, stackmember * candidat srv.precedence = 1; - stack[(*depth)++] = srv; + stackpush(stack, srv, depth); /* we're out of way, try adding now */ if (!addmember(stack, depth, candidate)) @@ -517,7 +534,7 @@ evalmorethan(stackmember * stack, int *depth, int whereAmI, stackmember * candid srv.precedence = 1; - stack[(*depth)++] = srv; + stackpush(stack, srv, depth); /* we're out of way, try adding now */ if (!addmember(stack, depth, candidate)) @@ -566,7 +583,7 @@ evalequals(stackmember * stack, int *depth, int whereAmI, srv.precedence = 1; - stack[(*depth)++] = srv; + stackpush(stack, srv, depth); /* we're out of way, try adding now */ if (!addmember(stack, depth, candidate)) @@ -613,7 +630,7 @@ evalnotequals(stackmember * stack, int *depth, int whereAmI, stackmember * candi srv.precedence = 1; - stack[(*depth)++] = srv; + stackpush(stack, srv, depth); /* we're out of way, try adding now */ if (!addmember(stack, depth, candidate)) @@ -953,6 +970,9 @@ addmember(stackmember * stack, int *stackdepth, stackmember * candidate) /* !(!(a==b))) is why thats safe */ /* strictly less than until we unwind */ + if (*stackdepth >= ESI_STACK_DEPTH_LIMIT) + throw Esi::Error("ESI expression too complex to add member"); + if (candidate->precedence < stack[*stackdepth - 1].precedence || candidate->precedence < stack[*stackdepth - 2].precedence) { /* must be an operator */ @@ -968,10 +988,10 @@ addmember(stackmember * stack, int *stackdepth, stackmember * candidate) return 0; } } else { - stack[(*stackdepth)++] = *candidate; + stackpush(stack, *candidate, stackdepth); } } else if (candidate->valuetype != ESI_EXPR_INVALID) - stack[(*stackdepth)++] = *candidate; + stackpush(stack, *candidate, stackdepth); return 1; } @@ -979,7 +999,7 @@ addmember(stackmember * stack, int *stackdepth, stackmember * candidate) int ESIExpression::Evaluate(char const *s) { - stackmember stack[20]; + stackmember stack[ESI_STACK_DEPTH_LIMIT]; int stackdepth = 0; char const *end; PROF_start(esiExpressionEval);