Skip to content

Commit

Permalink
[logging] Downgrade hapi connection errors when connecting with the w… (
Browse files Browse the repository at this point in the history
#11209)

* [logging] Downgrade hapi connection errors when connecting with the wrong protocol

* [logging] Check for error code instead of message for requesting https when serving http errors
  • Loading branch information
jbudz authored and kimjoar committed Jul 10, 2017
1 parent 941e57f commit 00cd9ea
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 15 deletions.
46 changes: 37 additions & 9 deletions src/server/logging/__tests__/log_interceptor.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@ import expect from 'expect.js';

import { LogInterceptor } from '../log_interceptor';

function stubClientErrorEvent(errno) {
function stubClientErrorEvent(errorMeta) {
const error = new Error();
error.errno = errno;

Object.assign(error, errorMeta);
return {
event: 'error',
pid: 1234,
Expand All @@ -15,9 +14,9 @@ function stubClientErrorEvent(errno) {
};
}

const stubEconnresetEvent = () => stubClientErrorEvent('ECONNRESET');
const stubEpipeEvent = () => stubClientErrorEvent('EPIPE');
const stubEcanceledEvent = () => stubClientErrorEvent('ECANCELED');
const stubEconnresetEvent = () => stubClientErrorEvent({ errno: 'ECONNRESET' });
const stubEpipeEvent = () => stubClientErrorEvent({ errno: 'EPIPE' });
const stubEcanceledEvent = () => stubClientErrorEvent({ errno: 'ECANCELED' });

function assertDowngraded(transformed) {
expect(!!transformed).to.be(true);
Expand All @@ -43,7 +42,7 @@ describe('server logging LogInterceptor', () => {

it('ignores non ECONNRESET events', () => {
const interceptor = new LogInterceptor();
const event = stubClientErrorEvent('not ECONNRESET');
const event = stubClientErrorEvent({ errno: 'not ECONNRESET' });
expect(interceptor.downgradeIfEconnreset(event)).to.be(null);
});

Expand Down Expand Up @@ -71,7 +70,7 @@ describe('server logging LogInterceptor', () => {

it('ignores non EPIPE events', () => {
const interceptor = new LogInterceptor();
const event = stubClientErrorEvent('not EPIPE');
const event = stubClientErrorEvent({ errno: 'not EPIPE' });
expect(interceptor.downgradeIfEpipe(event)).to.be(null);
});

Expand Down Expand Up @@ -99,7 +98,7 @@ describe('server logging LogInterceptor', () => {

it('ignores non ECANCELED events', () => {
const interceptor = new LogInterceptor();
const event = stubClientErrorEvent('not ECANCELED');
const event = stubClientErrorEvent({ errno: 'not ECANCELLED' });
expect(interceptor.downgradeIfEcanceled(event)).to.be(null);
});

Expand All @@ -110,4 +109,33 @@ describe('server logging LogInterceptor', () => {
expect(interceptor.downgradeIfEcanceled(event)).to.be(null);
});
});

describe('#downgradeIfHTTPSWhenHTTP', () => {
it('transforms https requests when serving http errors', () => {
const interceptor = new LogInterceptor();
const event = stubClientErrorEvent({ message: 'Parse Error', code: 'HPE_INVALID_METHOD' });
assertDowngraded(interceptor.downgradeIfHTTPSWhenHTTP(event));
});

it('ignores non events', () => {
const interceptor = new LogInterceptor();
const event = stubClientErrorEvent({ message: 'Parse Error', code: 'NOT_HPE_INVALID_METHOD' });
expect(interceptor.downgradeIfEcanceled(event)).to.be(null);
});
});

describe('#downgradeIfHTTPWhenHTTPS', () => {
it('transforms http requests when serving https errors', () => {
const message = '40735139278848:error:1407609C:SSL routines:SSL23_GET_CLIENT_HELLO:http request:../deps/openssl/openssl/ssl/s23_srvr.c:394'; // eslint-disable-line max-len
const interceptor = new LogInterceptor();
const event = stubClientErrorEvent({ message });
assertDowngraded(interceptor.downgradeIfHTTPWhenHTTPS(event));
});

it('ignores non events', () => {
const interceptor = new LogInterceptor();
const event = stubClientErrorEvent({ message: 'Not error' });
expect(interceptor.downgradeIfEcanceled(event)).to.be(null);
});
});
});
46 changes: 40 additions & 6 deletions src/server/logging/log_interceptor.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
import Stream from 'stream';
import { get, isEqual } from 'lodash';

const GET_CLIENT_HELLO = /GET_CLIENT_HELLO:http/;

function doTagsMatch(event, tags) {
return isEqual(get(event, 'tags'), tags);
}

function doesMessageMatch(errorMessage, match) {
if (!errorMessage) return false;
const isRegExp = match instanceof RegExp;
if (isRegExp) return match.test(errorMessage);
return errorMessage === match;
}
// converts the given event into a debug log if it's an error of the given type
function downgradeIfErrorMatches(errorType, event) {
function downgradeIfErrorType(errorType, event, field = 'errno') {
const isClientError = doTagsMatch(event, ['connection', 'client', 'error']);
const matchesErrorType = isClientError && get(event, 'data.errno') === errorType;
const matchesErrorType = isClientError && get(event, `data.${field}`) === errorType;

if (!matchesErrorType) return null;

Expand All @@ -23,6 +31,22 @@ function downgradeIfErrorMatches(errorType, event) {
};
}

function downgradeIfErrorMessage(match, event) {
const isClientError = doTagsMatch(event, ['connection', 'client', 'error']);
const errorMessage = get(event, 'data.message');
const matchesErrorMessage = isClientError && doesMessageMatch(errorMessage, match);

if (!matchesErrorMessage) return null;

return {
event: 'log',
pid: event.pid,
timestamp: event.timestamp,
tags: ['debug', 'connection'],
data: errorMessage
};
}

export class LogInterceptor extends Stream.Transform {
constructor() {
super({
Expand All @@ -42,7 +66,7 @@ export class LogInterceptor extends Stream.Transform {
* @param {object} - log event
*/
downgradeIfEconnreset(event) {
return downgradeIfErrorMatches('ECONNRESET', event);
return downgradeIfErrorType('ECONNRESET', event);
}

/**
Expand All @@ -56,7 +80,7 @@ export class LogInterceptor extends Stream.Transform {
* @param {object} - log event
*/
downgradeIfEpipe(event) {
return downgradeIfErrorMatches('EPIPE', event);
return downgradeIfErrorType('EPIPE', event);
}

/**
Expand All @@ -70,13 +94,23 @@ export class LogInterceptor extends Stream.Transform {
* @param {object} - log event
*/
downgradeIfEcanceled(event) {
return downgradeIfErrorMatches('ECANCELED', event);
return downgradeIfErrorType('ECANCELED', event);
}

downgradeIfHTTPSWhenHTTP(event) {
return downgradeIfErrorType('HPE_INVALID_METHOD', event, 'code');
}

downgradeIfHTTPWhenHTTPS(event) {
return downgradeIfErrorMessage(GET_CLIENT_HELLO, event);
}

_transform(event, enc, next) {
const downgraded = this.downgradeIfEconnreset(event)
|| this.downgradeIfEpipe(event)
|| this.downgradeIfEcanceled(event);
|| this.downgradeIfEcanceled(event)
|| this.downgradeIfHTTPSWhenHTTP(event)
|| this.downgradeIfHTTPWhenHTTPS(event);

this.push(downgraded || event);
next();
Expand Down

0 comments on commit 00cd9ea

Please sign in to comment.