Skip to content

Commit

Permalink
Merge pull request #3773 from DataDog/vzakharov/appsec_npe_hotfix
Browse files Browse the repository at this point in the history
Fixed AppSec NullPointerException for non-web spans.
  • Loading branch information
ValentinZakharov authored Aug 31, 2022
2 parents 40ff16b + e8c619d commit 8016f99
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ public void init() {
events.requestEnded(),
(RequestContext ctx_, IGSpanInfo spanInfo) -> {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
if (ctx == null) {
return NoopFlow.INSTANCE;
}

producerService.publishEvent(ctx, EventType.REQUEST_END);

TraceSegment traceSeg = ctx_.getTraceSegment();
Expand Down Expand Up @@ -171,6 +175,10 @@ public void init() {
EVENTS.requestBodyStart(),
(RequestContext ctx_, StoredBodySupplier supplier) -> {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
if (ctx == null) {
return null;
}

ctx.setStoredRequestBodySupplier(supplier);
producerService.publishEvent(ctx, EventType.REQUEST_BODY_START);
return null;
Expand All @@ -182,6 +190,10 @@ public void init() {
EVENTS.requestPathParams(),
(ctx_, data) -> {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
if (ctx == null) {
return NoopFlow.INSTANCE;
}

if (ctx.isPathParamsPublished()) {
log.debug("Second or subsequent publication of request params");
return NoopFlow.INSTANCE;
Expand All @@ -201,8 +213,7 @@ public void init() {
EVENTS.requestBodyDone(),
(RequestContext ctx_, StoredBodySupplier supplier) -> {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);

if (ctx.isRawReqBodyPublished()) {
if (ctx == null || ctx.isRawReqBodyPublished()) {
return NoopFlow.INSTANCE;
}
ctx.setRawReqBodyPublished(true);
Expand Down Expand Up @@ -232,6 +243,10 @@ public void init() {
EVENTS.requestBodyProcessed(),
(RequestContext ctx_, Object obj) -> {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
if (ctx == null) {
return NoopFlow.INSTANCE;
}

if (ctx.isConvertedReqBodyPublished()) {
log.debug(
"Request body already published; will ignore new value of type {}",
Expand All @@ -258,7 +273,7 @@ public void init() {
EVENTS.requestClientSocketAddress(),
(ctx_, ip, port) -> {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
if (ctx.isReqDataPublished()) {
if (ctx == null || ctx.isReqDataPublished()) {
return NoopFlow.INSTANCE;
}
ctx.setPeerAddress(ip);
Expand All @@ -270,7 +285,7 @@ public void init() {
EVENTS.responseStarted(),
(ctx_, status) -> {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
if (ctx.isRespDataPublished()) {
if (ctx == null || ctx.isRespDataPublished()) {
return NoopFlow.INSTANCE;
}
ctx.setResponseStatus(status);
Expand All @@ -286,7 +301,7 @@ public void init() {
EVENTS.responseHeaderDone(),
ctx_ -> {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
if (ctx.isRespDataPublished()) {
if (ctx == null || ctx.isRespDataPublished()) {
return NoopFlow.INSTANCE;
}
ctx.finishResponseHeaders();
Expand All @@ -297,6 +312,10 @@ public void init() {
EVENTS.grpcServerRequestMessage(),
(ctx_, obj) -> {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
if (ctx == null) {
return NoopFlow.INSTANCE;
}

if (grpcServerRequestMsgSubInfo == null) {
grpcServerRequestMsgSubInfo =
producerService.getDataSubscribers(KnownAddresses.GRPC_SERVER_REQUEST_MESSAGE);
Expand Down Expand Up @@ -335,6 +354,10 @@ private static class NewRequestHeaderCallback
@Override
public void accept(RequestContext ctx_, String name, String value) {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
if (ctx == null) {
return;
}

if (name.equalsIgnoreCase("cookie")) {
Map<String, List<String>> cookies = CookieCutter.parseCookieHeader(value);
ctx.addCookies(cookies);
Expand All @@ -347,7 +370,7 @@ public void accept(RequestContext ctx_, String name, String value) {
private class RequestHeadersDoneCallback implements Function<RequestContext, Flow<Void>> {
public Flow<Void> apply(RequestContext ctx_) {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
if (ctx.isReqDataPublished()) {
if (ctx == null || ctx.isReqDataPublished()) {
return NoopFlow.INSTANCE;
}
ctx.finishRequestHeaders();
Expand All @@ -360,6 +383,10 @@ private class MethodAndRawURICallback
@Override
public Flow<Void> apply(RequestContext ctx_, String method, URIDataAdapter uri) {
AppSecRequestContext ctx = ctx_.getData(RequestContextSlot.APPSEC);
if (ctx == null) {
return NoopFlow.INSTANCE;
}

if (ctx.isReqDataPublished()) {
log.debug(
"Request method and URI already published; will ignore new values {}, {}", method, uri);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -673,4 +673,51 @@ class GatewayBridgeSpecification extends DDSpecification {
then:
1 * pp.processTraceSegment(traceSegment, ctx.data, [])
}

void 'no appsec events if was not created request context in request_start event'() {
RequestContext emptyCtx = new RequestContext() {
final Object data = null

@Override
Object getData(RequestContextSlot slot) {
data
}

@Override
final TraceSegment getTraceSegment() {
GatewayBridgeSpecification.this.traceSegment
}

@Override
void close() throws IOException {}
}

StoredBodySupplier supplier = Mock()
IGSpanInfo spanInfo = Mock(AgentSpan)
Object obj = 'obj'

when:
// request start event doesn't happen and not create AppSecRequestContext
def flowMethod = requestMethodURICB.apply(emptyCtx, 'GET', TestURIDataAdapter.create('/a'))
def flowParams = pathParamsCB.apply(emptyCtx, [a: 'b'])
def flowSock = requestSocketAddressCB.apply(emptyCtx, '0.0.0.0', 5555)
reqHeaderCB.accept(emptyCtx, 'header_name', 'header_value')
def flowHeadersDone = reqHeadersDoneCB.apply(emptyCtx)
requestBodyStartCB.apply(emptyCtx, supplier)
def flowBodyEnd = requestBodyDoneCB.apply(emptyCtx, supplier)
def flowBodyProc = requestBodyProcessedCB.apply(emptyCtx, obj)
def flowReqEnd = requestEndedCB.apply(emptyCtx, spanInfo)
def appSecReqCtx = emptyCtx.getData(RequestContextSlot.APPSEC)

then:
appSecReqCtx == null
flowMethod == NoopFlow.INSTANCE
flowParams == NoopFlow.INSTANCE
flowSock == NoopFlow.INSTANCE
flowHeadersDone == NoopFlow.INSTANCE
flowBodyEnd == NoopFlow.INSTANCE
flowBodyProc == NoopFlow.INSTANCE
flowReqEnd == NoopFlow.INSTANCE
0 * _
}
}

0 comments on commit 8016f99

Please sign in to comment.