From e65a30ddaaf027855b57355fe35b88b97053ac99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Fernando=20Mu=C3=B1oz=20Mej=C3=ADas?= Date: Tue, 28 Apr 2009 21:14:50 +0200 Subject: Add licensing information. I'm not sure if GPLv3 contemplates the ability to link to proprietary software, if it was previous work. I explicitly allow linking to OCI. --- plugins/omoracle/omoracle.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'plugins/omoracle/omoracle.c') diff --git a/plugins/omoracle/omoracle.c b/plugins/omoracle/omoracle.c index 71cc8e1f..3a1f141f 100644 --- a/plugins/omoracle/omoracle.c +++ b/plugins/omoracle/omoracle.c @@ -43,6 +43,10 @@ need to define the properties on the template in the correct order you want them passed to the statement! + This file is licensed under the terms of the GPL version 3 or, at + your choice, any later version. Exceptionally (perhaps), you are + allowed to link to the Oracle Call Interface in your derived work + Author: Luis Fernando Muñoz Mejías -- cgit v1.2.3 From 65a69831e955fa32b23e8f25d3ba67b1e3058e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Fernando=20Mu=C3=B1oz=20Mej=C3=ADas?= Date: Tue, 28 Apr 2009 21:14:51 +0200 Subject: Add the $OmoracleBatchItemSize directive This directive controls the amount of memory needed for properties in the batch. Users should specify the largest value they expect in the statement. As per Rainer's comment: on MAX_BUFSIZE: I'd tend to make this configurable, because with RFC5424 messages can be much longer and RFC5425 now recommends a minimum maximum size of 8K. So we let users to choose. Maybe we need a sensible default value to make users' lifes easier? Also, the old non-vector based interface is not supported anymore. I broke it already when moving to this stage. --- plugins/omoracle/omoracle.c | 46 ++++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 17 deletions(-) (limited to 'plugins/omoracle/omoracle.c') diff --git a/plugins/omoracle/omoracle.c b/plugins/omoracle/omoracle.c index 3a1f141f..d6349d89 100644 --- a/plugins/omoracle/omoracle.c +++ b/plugins/omoracle/omoracle.c @@ -21,6 +21,13 @@ $OmoracleBatchSize: Number of elements to send to the DB on each transaction. + $OmoracleBatchItemSize: Number of characters each property may + have. Make it as big as the longest value you expect for *any* + property in the sentence. For instance, if you expect 5 arguments + to the statement, 4 have 10 bytes and the 5th may be up to 3KB, + then specify $OmoracleBatchItemSize 3072. Please, remember to + leave space to the trailing \0!! + $OmoracleStatement: Statement to be prepared and executed in batches. Please note that Oracle's prepared statements have their placeholders as ':identifier', and this module uses the colon to @@ -89,6 +96,8 @@ struct oracle_batch int n; /* Number of arguments the statement takes */ int arguments; + /** Maximum size of each parameter */ + int param_size; /* Parameters to pass to the statement on this transaction */ char*** parameters; /* Binding parameters */ @@ -125,12 +134,10 @@ static char* db_user; static char* db_password; /** Batch size. */ static int batch_size; +/** Size of each element in the batch. */ +static int batch_item_size; /** Statement to prepare and execute */ static char* db_statement; -/** Whether or not the core supports the newer array interface. The - * module is able to work in both modes, but the newer is the - * recommended one for performance reasons. */ -static int array_passing; /** Generic function for handling errors from OCI. @@ -248,8 +255,7 @@ static int prepare_statement(instanceData* pData) OCIBindByPos(pData->statement, pData->batch.bindings+i, pData->error, i+1, NULL, - MAX_BUFSIZE * - sizeof ***pData->batch.parameters, + pData->batch.param_size, SQLT_STR, NULL, NULL, NULL, 0, 0, OCI_DATA_AT_EXEC)); CHECKERR(pData->error, @@ -290,6 +296,7 @@ CODESTARTcreateInstance pData->batch.n = 0; pData->batch.size = batch_size; + pData->batch.param_size = batch_item_size * sizeof ***pData->batch.parameters; pData->batch.arguments = count_bind_parameters(pData->txt_statement); /* I know, this can be done with a single malloc() call but this is @@ -302,15 +309,14 @@ CODESTARTcreateInstance sizeof **pData->batch.parameters); CHKmalloc(pData->batch.parameters[i]); for (j = 0; j < pData->batch.size; j++) { - /* Each entry has at most MAX_BUFSIZE bytes - * because OCI doesn't like null-terminated - * strings when operating with batches, and - * the maximum size of each entry must be - * provided when binding - * parameters. MAX_BUFSIZE is long enough for - * usual entries. */ - pData->batch.parameters[i][j] = calloc(MAX_BUFSIZE, - sizeof ***pData->batch.parameters); + /* Each entry has at most + * pData->batch.param_size bytes because OCI + * doesn't like null-terminated strings when + * operating with batches, and the maximum + * size of each entry must be provided when + * binding parameters. pData->batch.param_size + * is long enough for usual entries. */ + pData->batch.parameters[i][j] = malloc(pData->batch.param_size); CHKmalloc(pData->batch.parameters[i][j]); } } @@ -480,7 +486,7 @@ CODESTARTdoAction for (i = 0; i < pData->batch.arguments && params[i]; i++) { dbgprintf("batch[%d][%d]=%s\n", i, pData->batch.n, params[i]); strncpy(pData->batch.parameters[i][pData->batch.n], params[i], - MAX_BUFSIZE); + pData->batch.param_size); CHKmalloc(pData->batch.parameters[i][pData->batch.n]); } pData->batch.n++; @@ -520,6 +526,7 @@ resetConfigVariables(uchar __attribute__((unused)) *pp, if (db_statement != NULL) free(db_statement); db_name = db_user = db_password = db_statement = NULL; + batch_size = batch_item_size = 0; RETiRet; } @@ -549,6 +556,7 @@ CODEmodInit_QueryRegCFSLineHdlr CHKiRet(omsdRegCFSLineHdlr((uchar*) "resetconfigvariables", 1, eCmdHdlrCustomHandler, resetConfigVariables, NULL, STD_LOADABLE_MODULE_ID)); + CHKiRet(omsdRegCFSLineHdlr((uchar*) "omoracledbuser", 0, eCmdHdlrGetWord, NULL, &db_user, STD_LOADABLE_MODULE_ID)); @@ -563,11 +571,15 @@ CODEmodInit_QueryRegCFSLineHdlr STD_LOADABLE_MODULE_ID)); CHKiRet(pHostQueryEtryPt((uchar*)"OMSRgetSupportedTplOpts", &supported_options)); CHKiRet((*supported_options)(&opts)); - if (!(array_passing = opts & OMSR_TPL_AS_ARRAY)) + if (!(opts & OMSR_TPL_AS_ARRAY)) ABORT_FINALIZE(RS_RET_RSCORE_TOO_OLD); CHKiRet(omsdRegCFSLineHdlr((uchar*) "omoraclestatement", 0, eCmdHdlrCustomHandler, get_db_statement, &db_statement, STD_LOADABLE_MODULE_ID)); + CHKiRet(omsdRegCFSLineHdlr((uchar*) "omoraclebatchitemsize", 0, + eCmdHdlrInt, NULL, + &batch_item_size, STD_LOADABLE_MODULE_ID)); + ENDmodInit -- cgit v1.2.3 From c35ce31aed4d873ed9564332374fbf82d7ff187d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Fernando=20Mu=C3=B1oz=20Mej=C3=ADas?= Date: Tue, 28 Apr 2009 21:14:52 +0200 Subject: Replace get_db_statement by a template. Instead of reading a complete line, we'll use a template and delegate in the core to read such template. Then, all omoracle has to do is to find that template and use it as the prepared statement. I'm not sure if this is the correct approach, though. It has to dig too much into rsyslog's structures... txt_statement is stored in a private area, so that we don't mess too much with rsyslog's internals (I still don't feel comfortable with this much digging into template structures). --- plugins/omoracle/omoracle.c | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-) (limited to 'plugins/omoracle/omoracle.c') diff --git a/plugins/omoracle/omoracle.c b/plugins/omoracle/omoracle.c index d6349d89..d1b3b955 100644 --- a/plugins/omoracle/omoracle.c +++ b/plugins/omoracle/omoracle.c @@ -28,10 +28,11 @@ then specify $OmoracleBatchItemSize 3072. Please, remember to leave space to the trailing \0!! - $OmoracleStatement: Statement to be prepared and executed in - batches. Please note that Oracle's prepared statements have their - placeholders as ':identifier', and this module uses the colon to - guess how many placeholders there will be. + $OmoracleStatementTemplate: Name of the template containing the + statement to be prepared and executed in batches. Please note that + Oracle's prepared statements have their placeholders as + ':identifier', and this module uses the colon to guess how many + placeholders there will be. All these directives are mandatory. The dbstring can be an Oracle easystring or a DB name, as present in the tnsnames.ora file. @@ -273,6 +274,7 @@ finalize_it: /* Resource allocation */ BEGINcreateInstance int i, j; + struct template* tpl; CODESTARTcreateInstance ASSERT(pData != NULL); @@ -289,14 +291,16 @@ CODESTARTcreateInstance CHECKENV(pData->environment, OCIHandleAlloc(pData->environment, (void*) &(pData->statement), OCI_HTYPE_STMT, 0, NULL)); - pData->txt_statement = strdup(db_statement); + tpl = tplFind(db_statement, strlen(db_statement)); + pData->txt_statement = strdup(tpl->pEntryRoot->data.constant.pConstant); CHKmalloc(pData->txt_statement); dbgprintf("omoracle will run stored statement: %s\n", pData->txt_statement); pData->batch.n = 0; pData->batch.size = batch_size; - pData->batch.param_size = batch_item_size * sizeof ***pData->batch.parameters; + pData->batch.param_size = batch_item_size * + sizeof ***pData->batch.parameters; pData->batch.arguments = count_bind_parameters(pData->txt_statement); /* I know, this can be done with a single malloc() call but this is @@ -439,7 +443,6 @@ finalize_it: BEGINisCompatibleWithFeature CODESTARTisCompatibleWithFeature /* Right now, this module is compatible with nothing. */ - dbgprintf ("***** OMORACLE ***** At isCompatibleWithFeature\n"); iRet = RS_RET_INCOMPATIBLE; ENDisCompatibleWithFeature @@ -465,6 +468,7 @@ CODE_STD_STRING_REQUESTparseSelectorAct(1); CHKiRet(createInstance(&pData)); CHKmalloc(pData->connection = strdup(db_name)); CHKiRet(startSession(pData, db_name, db_user, db_password)); + CHKiRet(prepare_statement(pData)); dbgprintf ("omoracle module got all its resources allocated " @@ -530,22 +534,6 @@ resetConfigVariables(uchar __attribute__((unused)) *pp, RETiRet; } -/** As I don't find any handler that reads an entire line, I write my - * own. */ -static int get_db_statement(char** line, char** stmt) -{ - DEFiRet; - - while (isspace(**line)) - (*line)++; - dbgprintf ("Config line: %s\n", *line); - *stmt = strdup(*line); - CHKmalloc(*stmt); - dbgprintf ("Statement: %s\n", *stmt); -finalize_it: - RETiRet; -} - BEGINmodInit() rsRetVal (*supported_options)(unsigned long *pOpts); unsigned long opts; @@ -574,8 +562,8 @@ CODEmodInit_QueryRegCFSLineHdlr if (!(opts & OMSR_TPL_AS_ARRAY)) ABORT_FINALIZE(RS_RET_RSCORE_TOO_OLD); - CHKiRet(omsdRegCFSLineHdlr((uchar*) "omoraclestatement", 0, - eCmdHdlrCustomHandler, get_db_statement, + CHKiRet(omsdRegCFSLineHdlr((uchar*) "omoraclestatementtemplate", 0, + eCmdHdlrGetWord, NULL, &db_statement, STD_LOADABLE_MODULE_ID)); CHKiRet(omsdRegCFSLineHdlr((uchar*) "omoraclebatchitemsize", 0, -- cgit v1.2.3 From 9823c73d1d07e50e6bec7f7c02c88d61d6955526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Fernando=20Mu=C3=B1oz=20Mej=C3=ADas?= Date: Tue, 28 Apr 2009 21:14:53 +0200 Subject: Make it recover from errors on insertions. If the database rejected some entry, making the statement fail on it, the batch was not cleaned and the same values were retried over and over, causing a cascade of failures and a denial of service. We use now OCI_BATCH_ERRORS so that everything valid in the batch is inserted, and rejected values can be discarded. --- plugins/omoracle/omoracle.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'plugins/omoracle/omoracle.c') diff --git a/plugins/omoracle/omoracle.c b/plugins/omoracle/omoracle.c index d1b3b955..331b7dd4 100644 --- a/plugins/omoracle/omoracle.c +++ b/plugins/omoracle/omoracle.c @@ -342,13 +342,12 @@ static int insert_to_db(instanceData* pData) OCIStmtExecute(pData->service, pData->statement, pData->error, - pData->batch.n, 0, NULL, NULL, OCI_DEFAULT)); + pData->batch.n, 0, NULL, NULL, + OCI_BATCH_ERRORS)); - CHECKERR(pData->error, - OCITransCommit(pData->service, pData->error, 0)); - - pData->batch.n = 0; finalize_it: + pData->batch.n = 0; + OCITransCommit(pData->service, pData->error, 0); dbgprintf ("omoracle insertion to DB %s\n", iRet == RS_RET_OK ? "succeeded" : "did not succeed"); RETiRet; -- cgit v1.2.3