|
|
bfd145 |
commit fdd4123629320aa1ee4c3481bb392437c90d188d
|
|
|
bfd145 |
Author: Amos Jeffries <yadij@users.noreply.github.com>
|
|
|
bfd145 |
Date: 2019-05-20 11:23:13 +0000
|
|
|
bfd145 |
|
|
|
bfd145 |
ESI: convert parse exceptions into 500 status response (#411)
|
|
|
bfd145 |
|
|
|
bfd145 |
Produce a valid HTTP 500 status reply and continue operations when
|
|
|
bfd145 |
ESI parser throws an exception. This will prevent incomplete ESI
|
|
|
bfd145 |
responses reaching clients on server errors. Such responses might
|
|
|
bfd145 |
have been cacheable and thus corrupted, albeit corrupted consistently
|
|
|
bfd145 |
and at source by the reverse-proxy delivering them.
|
|
|
bfd145 |
|
|
|
bfd145 |
ESI: throw on large stack recursions (#408)
|
|
|
bfd145 |
|
|
|
bfd145 |
This reduces the impact on concurrent clients to only those
|
|
|
bfd145 |
accessing the malformed resource.
|
|
|
bfd145 |
|
|
|
bfd145 |
Depending on what type of recursion is being performed the
|
|
|
bfd145 |
resource may appear to the client with missing segments, or
|
|
|
bfd145 |
not at all.
|
|
|
bfd145 |
|
|
|
bfd145 |
diff --git a/src/esi/Context.h b/src/esi/Context.h
|
|
|
bfd145 |
index f3281a1..1b08cfb 100644
|
|
|
bfd145 |
--- a/src/esi/Context.h
|
|
|
bfd145 |
+++ b/src/esi/Context.h
|
|
|
bfd145 |
@@ -12,6 +12,7 @@
|
|
|
bfd145 |
#include "clientStream.h"
|
|
|
bfd145 |
#include "err_type.h"
|
|
|
bfd145 |
#include "esi/Element.h"
|
|
|
bfd145 |
+#include "esi/Esi.h"
|
|
|
bfd145 |
#include "esi/Parser.h"
|
|
|
bfd145 |
#include "http/forward.h"
|
|
|
bfd145 |
#include "http/StatusCode.h"
|
|
|
bfd145 |
@@ -113,7 +114,7 @@ public:
|
|
|
bfd145 |
{
|
|
|
bfd145 |
|
|
|
bfd145 |
public:
|
|
|
bfd145 |
- ESIElement::Pointer stack[10]; /* a stack of esi elements that are open */
|
|
|
bfd145 |
+ ESIElement::Pointer stack[ESI_STACK_DEPTH_LIMIT]; /* a stack of esi elements that are open */
|
|
|
bfd145 |
int stackdepth; /* self explanatory */
|
|
|
bfd145 |
ESIParser::Pointer theParser;
|
|
|
bfd145 |
ESIElement::Pointer top();
|
|
|
bfd145 |
diff --git a/src/esi/Esi.cc b/src/esi/Esi.cc
|
|
|
bfd145 |
index cc662c4..e41d593 100644
|
|
|
bfd145 |
--- a/src/esi/Esi.cc
|
|
|
bfd145 |
+++ b/src/esi/Esi.cc
|
|
|
bfd145 |
@@ -29,6 +29,7 @@
|
|
|
bfd145 |
#include "esi/Expression.h"
|
|
|
bfd145 |
#include "esi/Segment.h"
|
|
|
bfd145 |
#include "esi/VarState.h"
|
|
|
bfd145 |
+#include "FadingCounter.h"
|
|
|
bfd145 |
#include "fatal.h"
|
|
|
bfd145 |
#include "http/Stream.h"
|
|
|
bfd145 |
#include "HttpHdrSc.h"
|
|
|
bfd145 |
@@ -930,13 +931,18 @@ void
|
|
|
bfd145 |
ESIContext::addStackElement (ESIElement::Pointer element)
|
|
|
bfd145 |
{
|
|
|
bfd145 |
/* Put on the stack to allow skipping of 'invalid' markup */
|
|
|
bfd145 |
- assert (parserState.stackdepth <11);
|
|
|
bfd145 |
+
|
|
|
bfd145 |
+ // throw an error if the stack location would be invalid
|
|
|
bfd145 |
+ if (parserState.stackdepth >= ESI_STACK_DEPTH_LIMIT)
|
|
|
bfd145 |
+ throw Esi::Error("ESI Too many nested elements");
|
|
|
bfd145 |
+ if (parserState.stackdepth < 0)
|
|
|
bfd145 |
+ throw Esi::Error("ESI elements stack error, probable error in ESI template");
|
|
|
bfd145 |
+
|
|
|
bfd145 |
assert (!failed());
|
|
|
bfd145 |
debugs(86, 5, "ESIContext::addStackElement: About to add ESI Node " << element.getRaw());
|
|
|
bfd145 |
|
|
|
bfd145 |
if (!parserState.top()->addElement(element)) {
|
|
|
bfd145 |
- debugs(86, DBG_IMPORTANT, "ESIContext::addStackElement: failed to add esi node, probable error in ESI template");
|
|
|
bfd145 |
- flags.error = 1;
|
|
|
bfd145 |
+ throw Esi::Error("ESIContext::addStackElement failed, probable error in ESI template");
|
|
|
bfd145 |
} else {
|
|
|
bfd145 |
/* added ok, push onto the stack */
|
|
|
bfd145 |
parserState.stack[parserState.stackdepth] = element;
|
|
|
bfd145 |
@@ -1188,13 +1194,10 @@ ESIContext::addLiteral (const char *s, int len)
|
|
|
bfd145 |
assert (len);
|
|
|
bfd145 |
debugs(86, 5, "literal length is " << len);
|
|
|
bfd145 |
/* give a literal to the current element */
|
|
|
bfd145 |
- assert (parserState.stackdepth <11);
|
|
|
bfd145 |
ESIElement::Pointer element (new esiLiteral (this, s, len));
|
|
|
bfd145 |
|
|
|
bfd145 |
- if (!parserState.top()->addElement(element)) {
|
|
|
bfd145 |
- debugs(86, DBG_IMPORTANT, "ESIContext::addLiteral: failed to add esi node, probable error in ESI template");
|
|
|
bfd145 |
- flags.error = 1;
|
|
|
bfd145 |
- }
|
|
|
bfd145 |
+ if (!parserState.top()->addElement(element))
|
|
|
bfd145 |
+ throw Esi::Error("ESIContext::addLiteral failed, probable error in ESI template");
|
|
|
bfd145 |
}
|
|
|
bfd145 |
|
|
|
bfd145 |
void
|
|
|
bfd145 |
@@ -1256,8 +1259,24 @@ ESIContext::parse()
|
|
|
bfd145 |
|
|
|
bfd145 |
PROF_start(esiParsing);
|
|
|
bfd145 |
|
|
|
bfd145 |
- while (buffered.getRaw() && !flags.error)
|
|
|
bfd145 |
- parseOneBuffer();
|
|
|
bfd145 |
+ try {
|
|
|
bfd145 |
+ while (buffered.getRaw() && !flags.error)
|
|
|
bfd145 |
+ parseOneBuffer();
|
|
|
bfd145 |
+
|
|
|
bfd145 |
+ } catch (Esi::ErrorDetail &errMsg) { // FIXME: non-const for c_str()
|
|
|
bfd145 |
+ // level-2: these are protocol/syntax errors from upstream
|
|
|
bfd145 |
+ debugs(86, 2, "WARNING: ESI syntax error: " << errMsg);
|
|
|
bfd145 |
+ setError();
|
|
|
bfd145 |
+ setErrorMessage(errMsg.c_str());
|
|
|
bfd145 |
+
|
|
|
bfd145 |
+ } catch (...) {
|
|
|
bfd145 |
+ // DBG_IMPORTANT because these are local issues the admin needs to fix
|
|
|
bfd145 |
+ static FadingCounter logEntries; // TODO: set horizon less than infinity
|
|
|
bfd145 |
+ if (logEntries.count(1) < 100)
|
|
|
bfd145 |
+ debugs(86, DBG_IMPORTANT, "ERROR: ESI parser: " << CurrentException);
|
|
|
bfd145 |
+ setError();
|
|
|
bfd145 |
+ setErrorMessage("ESI parser error");
|
|
|
bfd145 |
+ }
|
|
|
bfd145 |
|
|
|
bfd145 |
PROF_stop(esiParsing);
|
|
|
bfd145 |
|
|
|
bfd145 |
diff --git a/src/esi/Esi.h b/src/esi/Esi.h
|
|
|
bfd145 |
index 180b2c4..6fd5aac 100644
|
|
|
bfd145 |
--- a/src/esi/Esi.h
|
|
|
bfd145 |
+++ b/src/esi/Esi.h
|
|
|
bfd145 |
@@ -10,6 +10,11 @@
|
|
|
bfd145 |
#define SQUID_ESI_H
|
|
|
bfd145 |
|
|
|
bfd145 |
#include "clientStream.h"
|
|
|
bfd145 |
+#include "sbuf/SBuf.h"
|
|
|
bfd145 |
+
|
|
|
bfd145 |
+#if !defined(ESI_STACK_DEPTH_LIMIT)
|
|
|
bfd145 |
+#define ESI_STACK_DEPTH_LIMIT 20
|
|
|
bfd145 |
+#endif
|
|
|
bfd145 |
|
|
|
bfd145 |
/* ESI.c */
|
|
|
bfd145 |
extern CSR esiStreamRead;
|
|
|
bfd145 |
@@ -18,5 +23,14 @@ extern CSD esiStreamDetach;
|
|
|
bfd145 |
extern CSS esiStreamStatus;
|
|
|
bfd145 |
int esiEnableProcessing (HttpReply *);
|
|
|
bfd145 |
|
|
|
bfd145 |
+namespace Esi
|
|
|
bfd145 |
+{
|
|
|
bfd145 |
+
|
|
|
bfd145 |
+typedef SBuf ErrorDetail;
|
|
|
bfd145 |
+/// prepare an Esi::ErrorDetail for throw on ESI parser internal errors
|
|
|
bfd145 |
+inline Esi::ErrorDetail Error(const char *msg) { return ErrorDetail(msg); }
|
|
|
bfd145 |
+
|
|
|
bfd145 |
+} // namespace Esi
|
|
|
bfd145 |
+
|
|
|
bfd145 |
#endif /* SQUID_ESI_H */
|
|
|
bfd145 |
|
|
|
bfd145 |
diff --git a/src/esi/Expression.cc b/src/esi/Expression.cc
|
|
|
bfd145 |
index 2b5b762..8519b03 100644
|
|
|
bfd145 |
--- a/src/esi/Expression.cc
|
|
|
bfd145 |
+++ b/src/esi/Expression.cc
|
|
|
bfd145 |
@@ -10,6 +10,7 @@
|
|
|
bfd145 |
|
|
|
bfd145 |
#include "squid.h"
|
|
|
bfd145 |
#include "Debug.h"
|
|
|
bfd145 |
+#include "esi/Esi.h"
|
|
|
bfd145 |
#include "esi/Expression.h"
|
|
|
bfd145 |
#include "profiler/Profiler.h"
|
|
|
bfd145 |
|
|
|
bfd145 |
@@ -97,6 +98,17 @@ stackpop(stackmember * s, int *depth)
|
|
|
bfd145 |
cleanmember(&s[*depth]);
|
|
|
bfd145 |
}
|
|
|
bfd145 |
|
|
|
bfd145 |
+static void
|
|
|
bfd145 |
+stackpush(stackmember *stack, stackmember &item, int *depth)
|
|
|
bfd145 |
+{
|
|
|
bfd145 |
+ if (*depth < 0)
|
|
|
bfd145 |
+ throw Esi::Error("ESIExpression stack has negative size");
|
|
|
bfd145 |
+ if (*depth >= ESI_STACK_DEPTH_LIMIT)
|
|
|
bfd145 |
+ throw Esi::Error("ESIExpression stack is full, cannot push");
|
|
|
bfd145 |
+
|
|
|
bfd145 |
+ stack[(*depth)++] = item;
|
|
|
bfd145 |
+}
|
|
|
bfd145 |
+
|
|
|
bfd145 |
static evaluate evalnegate;
|
|
|
bfd145 |
static evaluate evalliteral;
|
|
|
bfd145 |
static evaluate evalor;
|
|
|
bfd145 |
@@ -208,6 +220,11 @@ evalnegate(stackmember * stack, int *depth, int whereAmI, stackmember * candidat
|
|
|
bfd145 |
/* invalid stack */
|
|
|
bfd145 |
return 1;
|
|
|
bfd145 |
|
|
|
bfd145 |
+ if (whereAmI < 0)
|
|
|
bfd145 |
+ throw Esi::Error("negate expression location too small");
|
|
|
bfd145 |
+ if (*depth >= ESI_STACK_DEPTH_LIMIT)
|
|
|
bfd145 |
+ throw Esi::Error("negate expression too complex");
|
|
|
bfd145 |
+
|
|
|
bfd145 |
if (stack[whereAmI + 1].valuetype != ESI_EXPR_EXPR)
|
|
|
bfd145 |
/* invalid operand */
|
|
|
bfd145 |
return 1;
|
|
|
bfd145 |
@@ -280,7 +297,7 @@ evalor(stackmember * stack, int *depth, int whereAmI, stackmember * candidate)
|
|
|
bfd145 |
|
|
|
bfd145 |
srv.precedence = 1;
|
|
|
bfd145 |
|
|
|
bfd145 |
- stack[(*depth)++] = srv;
|
|
|
bfd145 |
+ stackpush(stack, srv, depth);
|
|
|
bfd145 |
|
|
|
bfd145 |
/* we're out of way, try adding now */
|
|
|
bfd145 |
if (!addmember(stack, depth, candidate))
|
|
|
bfd145 |
@@ -327,7 +344,7 @@ evaland(stackmember * stack, int *depth, int whereAmI, stackmember * candidate)
|
|
|
bfd145 |
|
|
|
bfd145 |
srv.precedence = 1;
|
|
|
bfd145 |
|
|
|
bfd145 |
- stack[(*depth)++] = srv;
|
|
|
bfd145 |
+ stackpush(stack, srv, depth);
|
|
|
bfd145 |
|
|
|
bfd145 |
/* we're out of way, try adding now */
|
|
|
bfd145 |
if (!addmember(stack, depth, candidate))
|
|
|
bfd145 |
@@ -373,7 +390,7 @@ evallesseq(stackmember * stack, int *depth, int whereAmI, stackmember * candidat
|
|
|
bfd145 |
|
|
|
bfd145 |
srv.precedence = 1;
|
|
|
bfd145 |
|
|
|
bfd145 |
- stack[(*depth)++] = srv;
|
|
|
bfd145 |
+ stackpush(stack, srv, depth);
|
|
|
bfd145 |
|
|
|
bfd145 |
/* we're out of way, try adding now */
|
|
|
bfd145 |
if (!addmember(stack, depth, candidate))
|
|
|
bfd145 |
@@ -421,7 +438,7 @@ evallessthan(stackmember * stack, int *depth, int whereAmI, stackmember * candid
|
|
|
bfd145 |
|
|
|
bfd145 |
srv.precedence = 1;
|
|
|
bfd145 |
|
|
|
bfd145 |
- stack[(*depth)++] = srv;
|
|
|
bfd145 |
+ stackpush(stack, srv, depth);
|
|
|
bfd145 |
|
|
|
bfd145 |
/* we're out of way, try adding now */
|
|
|
bfd145 |
if (!addmember(stack, depth, candidate))
|
|
|
bfd145 |
@@ -469,7 +486,7 @@ evalmoreeq(stackmember * stack, int *depth, int whereAmI, stackmember * candidat
|
|
|
bfd145 |
|
|
|
bfd145 |
srv.precedence = 1;
|
|
|
bfd145 |
|
|
|
bfd145 |
- stack[(*depth)++] = srv;
|
|
|
bfd145 |
+ stackpush(stack, srv, depth);
|
|
|
bfd145 |
|
|
|
bfd145 |
/* we're out of way, try adding now */
|
|
|
bfd145 |
if (!addmember(stack, depth, candidate))
|
|
|
bfd145 |
@@ -517,7 +534,7 @@ evalmorethan(stackmember * stack, int *depth, int whereAmI, stackmember * candid
|
|
|
bfd145 |
|
|
|
bfd145 |
srv.precedence = 1;
|
|
|
bfd145 |
|
|
|
bfd145 |
- stack[(*depth)++] = srv;
|
|
|
bfd145 |
+ stackpush(stack, srv, depth);
|
|
|
bfd145 |
|
|
|
bfd145 |
/* we're out of way, try adding now */
|
|
|
bfd145 |
if (!addmember(stack, depth, candidate))
|
|
|
bfd145 |
@@ -566,7 +583,7 @@ evalequals(stackmember * stack, int *depth, int whereAmI,
|
|
|
bfd145 |
|
|
|
bfd145 |
srv.precedence = 1;
|
|
|
bfd145 |
|
|
|
bfd145 |
- stack[(*depth)++] = srv;
|
|
|
bfd145 |
+ stackpush(stack, srv, depth);
|
|
|
bfd145 |
|
|
|
bfd145 |
/* we're out of way, try adding now */
|
|
|
bfd145 |
if (!addmember(stack, depth, candidate))
|
|
|
bfd145 |
@@ -613,7 +630,7 @@ evalnotequals(stackmember * stack, int *depth, int whereAmI, stackmember * candi
|
|
|
bfd145 |
|
|
|
bfd145 |
srv.precedence = 1;
|
|
|
bfd145 |
|
|
|
bfd145 |
- stack[(*depth)++] = srv;
|
|
|
bfd145 |
+ stackpush(stack, srv, depth);
|
|
|
bfd145 |
|
|
|
bfd145 |
/* we're out of way, try adding now */
|
|
|
bfd145 |
if (!addmember(stack, depth, candidate))
|
|
|
bfd145 |
@@ -953,6 +970,9 @@ addmember(stackmember * stack, int *stackdepth, stackmember * candidate)
|
|
|
bfd145 |
/* !(!(a==b))) is why thats safe */
|
|
|
bfd145 |
/* strictly less than until we unwind */
|
|
|
bfd145 |
|
|
|
bfd145 |
+ if (*stackdepth >= ESI_STACK_DEPTH_LIMIT)
|
|
|
bfd145 |
+ throw Esi::Error("ESI expression too complex to add member");
|
|
|
bfd145 |
+
|
|
|
bfd145 |
if (candidate->precedence < stack[*stackdepth - 1].precedence ||
|
|
|
bfd145 |
candidate->precedence < stack[*stackdepth - 2].precedence) {
|
|
|
bfd145 |
/* must be an operator */
|
|
|
bfd145 |
@@ -968,10 +988,10 @@ addmember(stackmember * stack, int *stackdepth, stackmember * candidate)
|
|
|
bfd145 |
return 0;
|
|
|
bfd145 |
}
|
|
|
bfd145 |
} else {
|
|
|
bfd145 |
- stack[(*stackdepth)++] = *candidate;
|
|
|
bfd145 |
+ stackpush(stack, *candidate, stackdepth);
|
|
|
bfd145 |
}
|
|
|
bfd145 |
} else if (candidate->valuetype != ESI_EXPR_INVALID)
|
|
|
bfd145 |
- stack[(*stackdepth)++] = *candidate;
|
|
|
bfd145 |
+ stackpush(stack, *candidate, stackdepth);
|
|
|
bfd145 |
|
|
|
bfd145 |
return 1;
|
|
|
bfd145 |
}
|
|
|
bfd145 |
@@ -979,7 +999,7 @@ addmember(stackmember * stack, int *stackdepth, stackmember * candidate)
|
|
|
bfd145 |
int
|
|
|
bfd145 |
ESIExpression::Evaluate(char const *s)
|
|
|
bfd145 |
{
|
|
|
bfd145 |
- stackmember stack[20];
|
|
|
bfd145 |
+ stackmember stack[ESI_STACK_DEPTH_LIMIT];
|
|
|
bfd145 |
int stackdepth = 0;
|
|
|
bfd145 |
char const *end;
|
|
|
bfd145 |
PROF_start(esiExpressionEval);
|