From a7a261026a41f5c90386a8235ee43e9f9aaede93 Mon Sep 17 00:00:00 2001 From: Adam Dickmeiss Date: Mon, 9 Nov 2026 14:51:36 +0100 Subject: [PATCH] Validate server retrieval bounds and add regression tests Limit Present and piggyback retrieval to 10,000 records. Validate counts and start positions before narrowing to int, avoid overflow in range checks, and return failure status with rejection diagnostics. Allow requests whose start plus count equals INT_MAX. Add a yaz/test.h server test covering search, retrieval, record contents, piggyback behavior, invalid bounds, and the INT_MAX equality boundary. Register it with make check. Use odr_booldup and odr_intdup for allocation and initialization. Replace numeric diagnostics with named Bib-1 definitions. Refine ignore patterns for test binaries and generated test results, and remove trailing whitespace. Validation: all three src tests pass. --- include/yazpp/z-server.h | 2 +- src/.gitignore | 7 +- src/Makefile.am | 3 +- src/test_gdu.cpp | 2 +- src/test_server.cpp | 308 +++++++++++++++++++++++++++++++++++++++ src/yaz-z-server-sr.cpp | 113 ++++++++------ 6 files changed, 387 insertions(+), 48 deletions(-) create mode 100644 src/test_server.cpp diff --git a/include/yazpp/z-server.h b/include/yazpp/z-server.h index 7ce1776..e30a7f1 100644 --- a/include/yazpp/z-server.h +++ b/include/yazpp/z-server.h @@ -1,5 +1,5 @@ /* This file is part of the yazpp toolkit. - * Copyright (C) Index Data + * Copyright (C) Index Data * All rights reserved. * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: diff --git a/src/.gitignore b/src/.gitignore index 7806a0b..0a3034e 100644 --- a/src/.gitignore +++ b/src/.gitignore @@ -9,9 +9,12 @@ yazpp-config.in yaz-my-client yaz-my-server yaz-proxy -test_gdu* -test_query* +test_gdu +test_query +test_server *.lo *.o tstquery +*.log +*.trs *~ diff --git a/src/Makefile.am b/src/Makefile.am index 54ca7ba..20f3725 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,5 +1,5 @@ -check_PROGRAMS = test_query test_gdu +check_PROGRAMS = test_query test_gdu test_server noinst_PROGRAMS = yaz-my-server yaz-my-client bin_SCRIPTS = yazpp-config @@ -29,5 +29,6 @@ yaz_my_server_SOURCES=yaz-my-server.cpp yaz-marc-sample.cpp test_query_SOURCES=test_query.cpp test_gdu_SOURCES=test_gdu.cpp +test_server_SOURCES=test_server.cpp LDADD=libyazpp.la $(YAZLALIB) diff --git a/src/test_gdu.cpp b/src/test_gdu.cpp index c460d0f..63da65f 100644 --- a/src/test_gdu.cpp +++ b/src/test_gdu.cpp @@ -1,5 +1,5 @@ /* This file is part of the yazpp toolkit. - * Copyright (C) Index Data + * Copyright (C) Index Data * See the file LICENSE for details. */ diff --git a/src/test_server.cpp b/src/test_server.cpp new file mode 100644 index 0000000..e659415 --- /dev/null +++ b/src/test_server.cpp @@ -0,0 +1,308 @@ +/* This file is part of the yazpp toolkit. + * Copyright (C) Index Data + * See the file LICENSE for details. + */ + +#if HAVE_CONFIG_H +#include +#endif +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace yazpp_1; + +namespace { + +// Exercise BER decoding and encoding without opening a socket. +class Transport : public IPDU_Observable { + ODR m_odr; +public: + Z_APDU *response; + Transport() : m_odr(odr_createmem(ODR_DECODE)), response(0) {} + ~Transport() { odr_destroy(m_odr); } + void reset() { odr_reset(m_odr); response = 0; } + int send_PDU(const char *buf, int len) + { + odr_setbuf(m_odr, const_cast(buf), len, 0); + int ok = z_APDU(m_odr, &response, 0, 0); + YAZ_CHECK(ok); + return ok ? 0 : -1; + } + int connect(IPDU_Observer *, const char *) { return 0; } + int listen(IPDU_Observer *, const char *) { return 0; } + void shutdown() {} + IPDU_Observable *clone() { return new Transport; } + void destroy() {} + void idleTime(int) {} + const char *getpeername() { return "test"; } + void close_session() {} +}; + +class Server : public Z_Server { +public: + Server(Transport *transport) : Z_Server(transport) {} + void connectNotify() {} + void failNotify() { YAZ_CHECK(false); } + void timeoutNotify() { YAZ_CHECK(false); } + IPDU_Observer *sessionNotify(IPDU_Observable *, int) { return 0; } +}; + +class Retrieval : public Yaz_Facility_Retrieval { +public: + Odr_int hits; + int calls; + Retrieval() : hits(0), calls(0) {} + int sr_init(Z_InitRequest *, Z_InitResponse *) { return 1; } + void sr_search(Z_SearchRequest *req, Z_SearchResponse *res) + { + YAZ_CHECK(!strcmp(req->resultSetName, "test")); + *res->resultCount = hits; + } + void sr_present(Z_PresentRequest *, Z_PresentResponse *) {} + void sr_record(const char *set, int position, Odr_oid *, + Z_RecordComposition *, Z_NamePlusRecord *record, + Z_Records *) + { + calls++; + YAZ_CHECK(!strcmp(set, "test")); + char buf[40]; + sprintf(buf, "record %d", position); + create_databaseRecord(odr_encode(), record, "test", + yaz_oid_recsyn_sutrs, buf, strlen(buf)); + } +}; + +class Fixture { +public: + Retrieval retrieval; + Transport *transport; + Server server; + ODR out; + Fixture() : transport(new Transport), server(transport), + out(odr_createmem(ODR_ENCODE)) + { + server.facility_add(&retrieval, "retrieval"); + Z_APDU *req = zget_APDU(out, Z_APDU_initRequest); + *req->u.initRequest->preferredMessageSize = 16 * 1024 * 1024; + *req->u.initRequest->maximumRecordSize = 16 * 1024 * 1024; + Z_APDU *res = send(req); + YAZ_CHECK(res && res->which == Z_APDU_initResponse); + if (res && res->which == Z_APDU_initResponse) + YAZ_CHECK(*res->u.initResponse->result); + } + ~Fixture() { odr_destroy(out); } + Z_APDU *send(Z_APDU *req) + { + transport->reset(); + retrieval.calls = 0; + int ok = z_APDU(out, &req, 0, 0); + YAZ_CHECK(ok); + if (!ok) + return 0; + int len; + char *buf = odr_getbuf(out, &len, 0); + server.recv_PDU(buf, len); + odr_reset(out); + YAZ_CHECK(transport->response); + return transport->response; + } + Z_PresentResponse *present(Odr_int start, Odr_int count) + { + Z_APDU *req = zget_APDU(out, Z_APDU_presentRequest); + req->u.presentRequest->resultSetId = odr_strdup(out, "test"); + *req->u.presentRequest->resultSetStartPoint = start; + *req->u.presentRequest->numberOfRecordsRequested = count; + Z_APDU *res = send(req); + YAZ_CHECK(res && res->which == Z_APDU_presentResponse); + return res && res->which == Z_APDU_presentResponse ? + res->u.presentResponse : 0; + } + Z_SearchResponse *search(Odr_int hits, Odr_int small, + Odr_int large, Odr_int medium) + { + retrieval.hits = hits; + Z_APDU *apdu = zget_APDU(out, Z_APDU_searchRequest); + Z_SearchRequest *req = apdu->u.searchRequest; + req->resultSetName = odr_strdup(out, "test"); + req->num_databaseNames = 1; + req->databaseNames = (char **) odr_malloc(out, sizeof(char *)); + req->databaseNames[0] = odr_strdup(out, "test"); + req->query = (Z_Query *) odr_malloc(out, sizeof(Z_Query)); + req->query->which = Z_Query_type_2; + req->query->u.type_2 = odr_create_Odr_oct(out, "test", 4); + *req->smallSetUpperBound = small; + *req->largeSetLowerBound = large; + *req->mediumSetPresentNumber = medium; + Z_APDU *res = send(apdu); + YAZ_CHECK(res && res->which == Z_APDU_searchResponse); + return res && res->which == Z_APDU_searchResponse ? + res->u.searchResponse : 0; + } +}; + +void check_records(Z_Records *records, int start, int count) +{ + YAZ_CHECK(records && records->which == Z_Records_DBOSD); + if (!records || records->which != Z_Records_DBOSD) + return; + Z_NamePlusRecordList *list = records->u.databaseOrSurDiagnostics; + YAZ_CHECK_EQ(list->num_records, count); + for (int i = 0; i < list->num_records; i++) + { + Z_NamePlusRecord *rec = list->records[i]; + YAZ_CHECK(rec->which == Z_NamePlusRecord_databaseRecord); + if (rec->which != Z_NamePlusRecord_databaseRecord) + continue; + Z_External *ext = rec->u.databaseRecord; + YAZ_CHECK(ext && ext->which == Z_External_sutrs); + if (ext && ext->which == Z_External_sutrs) + { + char buf[40]; + sprintf(buf, "record %d", start + i); + YAZ_CHECK_EQ(ext->u.sutrs->len, strlen(buf)); + YAZ_CHECK(ext->u.sutrs->len == (int) strlen(buf) && + !memcmp(ext->u.sutrs->buf, buf, strlen(buf))); + } + } +} + +void check_diagnostic(Z_Records *records) +{ + YAZ_CHECK(records && records->which == Z_Records_NSD); + if (records && records->which == Z_Records_NSD) + YAZ_CHECK_EQ(*records->u.nonSurrogateDiagnostic->condition, + YAZ_BIB1_PRESENT_REQUEST_OUT_OF_RANGE); +} + +void test_present() +{ + Fixture f; + // Establish a result set, then retrieve records from a non-default offset. + f.search(10000, 0, 1, 0); + const int counts[] = {0, 1, 3, 10000}; + for (unsigned i = 0; i < sizeof(counts) / sizeof(*counts); i++) + { + int start = counts[i] == 10000 ? 1 : 5; + Z_PresentResponse *res = f.present(start, counts[i]); + if (!res) + continue; + YAZ_CHECK_EQ(*res->presentStatus, Z_PresentStatus_success); + YAZ_CHECK_EQ(*res->numberOfRecordsReturned, counts[i]); + YAZ_CHECK_EQ(*res->nextResultSetPosition, + counts[i] ? start + counts[i] : 0); + YAZ_CHECK_EQ(f.retrieval.calls, counts[i]); + check_records(res->records, start, counts[i]); + } + // The next position and final loop increment may safely reach INT_MAX. + f.search(INT_MAX, 0, 1, 0); + Z_PresentResponse *boundary = f.present(INT_MAX - 1, 1); + if (boundary) + { + YAZ_CHECK_EQ(*boundary->presentStatus, Z_PresentStatus_success); + YAZ_CHECK_EQ(*boundary->numberOfRecordsReturned, 1); + YAZ_CHECK_EQ(*boundary->nextResultSetPosition, INT_MAX); + YAZ_CHECK_EQ(f.retrieval.calls, 1); + check_records(boundary->records, INT_MAX - 1, 1); + } + const Odr_int max = std::numeric_limits::max(); + const Odr_int min = std::numeric_limits::min(); + const Odr_int invalid[][2] = { + {1, -1}, {1, min}, {1, 10001}, {1, max}, + {0, 1}, {-1, 1}, {min, 1}, {INT_MAX, 1}, + {max, 1}, {max, 10000}, {INT_MAX - 1, 2} + }; + for (unsigned i = 0; i < sizeof(invalid) / sizeof(*invalid); i++) + { + Z_PresentResponse *res = f.present(invalid[i][0], invalid[i][1]); + if (!res) + continue; + YAZ_CHECK_EQ(*res->presentStatus, Z_PresentStatus_failure); + YAZ_CHECK_EQ(*res->numberOfRecordsReturned, 0); + YAZ_CHECK_EQ(*res->nextResultSetPosition, 0); + YAZ_CHECK_EQ(f.retrieval.calls, 0); + check_diagnostic(res->records); + } +} + +void test_search() +{ + Fixture f; + // hits, small bound, large bound, medium count, expected returned count. + const Odr_int cases[][5] = { + {0, 5, 10, 3, 0}, {5, 5, 10, 3, 5}, + {6, 5, 10, 3, 3}, {6, 5, 10, 9, 6}, + {10, 5, 10, 3, 0}, {6, 5, 10, 0, 0}, + {10000, 10000, 10001, 0, 10000}, + {10001, 0, 10002, 10000, 10000} + }; + for (unsigned i = 0; i < sizeof(cases) / sizeof(*cases); i++) + { + const Odr_int *c = cases[i]; + Z_SearchResponse *res = f.search(c[0], c[1], c[2], c[3]); + if (!res) + continue; + YAZ_CHECK(*res->searchStatus); + YAZ_CHECK_EQ(*res->resultCount, c[0]); + YAZ_CHECK_EQ(*res->numberOfRecordsReturned, c[4]); + YAZ_CHECK_EQ(f.retrieval.calls, c[4]); + YAZ_CHECK_EQ(*res->nextResultSetPosition, + c[4] ? c[4] + 1 : (c[0] ? 1 : 0)); + if (c[4]) + { + YAZ_CHECK(res->presentStatus && + *res->presentStatus == Z_PresentStatus_success); + check_records(res->records, 1, (int) c[4]); + } + else + { + YAZ_CHECK(!res->presentStatus); + YAZ_CHECK(!res->records); + } + } + const Odr_int max = std::numeric_limits::max(); + const Odr_int invalid[][4] = { + // hits, smallSetUpperBound, largeSetLowerBound, mediumSetPresentNumber. + {6, -1, 10, 3}, {6, 5, -1, 3}, {6, 5, 10, -1}, + {10001, 10001, 10002, 0}, {10001, 0, 10002, 10001}, + {max, max, max, 0}, {max - 1, 0, max, max} + }; + for (unsigned i = 0; i < sizeof(invalid) / sizeof(*invalid); i++) + { + const Odr_int *c = invalid[i]; + Z_SearchResponse *res = f.search(c[0], c[1], c[2], c[3]); + if (!res) + continue; + YAZ_CHECK(*res->searchStatus); + YAZ_CHECK_EQ(*res->resultCount, c[0]); + YAZ_CHECK(res->presentStatus && + *res->presentStatus == Z_PresentStatus_failure); + YAZ_CHECK_EQ(*res->numberOfRecordsReturned, 0); + YAZ_CHECK_EQ(f.retrieval.calls, 0); + check_diagnostic(res->records); + } +} + +} + +int main(int argc, char **argv) +{ + YAZ_CHECK_INIT(argc, argv); + test_present(); + test_search(); + YAZ_CHECK_TERM; +} + +/* + * Local variables: + * c-basic-offset: 4 + * c-file-style: "Stroustrup" + * indent-tabs-mode: nil + * End: + * vim: shiftwidth=4 tabstop=8 expandtab + */ diff --git a/src/yaz-z-server-sr.cpp b/src/yaz-z-server-sr.cpp index f819f69..3646ab9 100644 --- a/src/yaz-z-server-sr.cpp +++ b/src/yaz-z-server-sr.cpp @@ -1,5 +1,5 @@ /* This file is part of the yazpp toolkit. - * Copyright (C) Index Data + * Copyright (C) Index Data * See the file LICENSE for details. */ @@ -7,10 +7,14 @@ #include #endif #include +#include #include +#include using namespace yazpp_1; +const int fetch_max = 10000; + Z_Records *Yaz_Facility_Retrieval::pack_records (Z_Server *s, const char *resultSetName, int start, int xnum, @@ -18,7 +22,8 @@ Z_Records *Yaz_Facility_Retrieval::pack_records (Z_Server *s, Odr_int *next, Odr_int *pres, Odr_oid *format) { - int recno, total_length = 0, toget = xnum, dumped_records = 0; + int total_length = 0, toget = xnum, dumped_records = 0; + Z_Records *records = (Z_Records *) odr_malloc (odr_encode(), sizeof(*records)); Z_NamePlusRecordList *reclist = @@ -33,7 +38,7 @@ Z_Records *Yaz_Facility_Retrieval::pack_records (Z_Server *s, *pres = Z_PresentStatus_success; *next = 0; - for (recno = start; reclist->num_records < toget; recno++) + for (int recno = start; reclist->num_records < toget; recno++) { Z_NamePlusRecord *this_rec = (Z_NamePlusRecord *) odr_malloc (odr_encode(), sizeof(*this_rec)); @@ -54,7 +59,8 @@ Z_Records *Yaz_Facility_Retrieval::pack_records (Z_Server *s, if (this_rec->which == Z_NamePlusRecord_databaseRecord && this_rec->u.databaseRecord == 0) { // handler did not return a record.. - create_surrogateDiagnostics(odr_encode(), this_rec, 0, 14, 0); + create_surrogateDiagnostics(odr_encode(), this_rec, + 0, YAZ_BIB1_SYSTEM_ERROR_IN_PRESENTING_RECORDS, 0); } /* * we get the number of bytes allocated on the stream before any @@ -75,7 +81,8 @@ Z_Records *Yaz_Facility_Retrieval::pack_records (Z_Server *s, { /* too big entirely */ reclist->records[reclist->num_records] = this_rec; create_surrogateDiagnostics(odr_encode(), this_rec, - this_rec->databaseName, 17, 0); + this_rec->databaseName, + YAZ_BIB1_RECORD_EXCEEDS_MAXIMUM_RECORD_SIZE, 0); reclist->num_records++; *next = recno + 1; dumped_records += this_length; @@ -88,8 +95,8 @@ Z_Records *Yaz_Facility_Retrieval::pack_records (Z_Server *s, yaz_log(YLOG_DEBUG, " Dropped it"); reclist->records[reclist->num_records] = this_rec; create_surrogateDiagnostics(odr_encode(), this_rec, - this_rec->databaseName, - 16, 0); + this_rec->databaseName, + YAZ_BIB1_RECORD_EXCEEDS_PREFERRED_MESSAGE_SIZE, 0); reclist->num_records++; // *next = freq.last_in_set ? 0 : recno + 1; *next = recno + 1; @@ -109,19 +116,21 @@ void Yaz_Facility_Retrieval::fetch_via_piggyback (Z_Server *s, Z_SearchRequest *req, Z_SearchResponse *res) { - bool_t *sr = (bool_t *)odr_malloc (odr_encode(), sizeof(*sr)); - *sr = 1; - - int toget = 0; + if (*req->smallSetUpperBound < 0 || + *req->mediumSetPresentNumber < 0 || + *req->largeSetLowerBound < 0) + { + res->records = create_nonSurrogateDiagnostics(odr_encode(), + YAZ_BIB1_PRESENT_REQUEST_OUT_OF_RANGE, 0); + res->presentStatus = odr_intdup(odr_encode(), Z_PresentStatus_failure); + return; + } + /* how many records does the user agent want, then? */ + Odr_int toget = 0; Z_RecordComposition comp, *compp = 0; - int hits = *res->resultCount; - - Odr_int *nulint = (Odr_int *)odr_malloc (odr_encode(), sizeof(*nulint)); - *nulint = 0; - comp.which = Z_RecordComp_simple; - /* how many records does the user agent want, then? */ + Odr_int hits = *res->resultCount; if (hits <= *req->smallSetUpperBound) { toget = hits; @@ -137,32 +146,33 @@ void Yaz_Facility_Retrieval::fetch_via_piggyback (Z_Server *s, compp = ∁ } - if (toget && !res->records) + if (toget > 0 && !res->records) { - res->presentStatus = (Odr_int *) - odr_malloc (odr_encode(), sizeof(Odr_int)); - *res->presentStatus = Z_PresentStatus_success; - res->records = - pack_records(s, req->resultSetName, 1, toget, compp, - res->nextResultSetPosition, - res->presentStatus, - req->preferredRecordSyntax); + res->presentStatus = odr_intdup(odr_encode(), Z_PresentStatus_success); + if (toget > fetch_max) + { + res->records = create_nonSurrogateDiagnostics(odr_encode(), + YAZ_BIB1_PRESENT_REQUEST_OUT_OF_RANGE, 0); + *res->presentStatus = Z_PresentStatus_failure; + } + else + { + res->records = + pack_records(s, req->resultSetName, 1, (int) toget, compp, + res->nextResultSetPosition, + res->presentStatus, + req->preferredRecordSyntax); + } if (!res->records) return; if (res->records->which == Z_Records_DBOSD) *res->numberOfRecordsReturned = res->records->u.databaseOrSurDiagnostics->num_records; - res->searchStatus = sr; - res->resultSetStatus = 0; } else { if (hits) *res->nextResultSetPosition = 1; - res->numberOfRecordsReturned = nulint; - res->searchStatus = sr; - res->resultSetStatus = 0; - res->presentStatus = 0; } } @@ -170,13 +180,30 @@ void Yaz_Facility_Retrieval::fetch_via_present (Z_Server *s, Z_PresentRequest *req, Z_PresentResponse *res) { + Odr_int toget = *req->numberOfRecordsRequested; + if (toget < 0 || toget > INT_MAX || toget > fetch_max) + { + res->records = create_nonSurrogateDiagnostics(odr_encode(), + YAZ_BIB1_PRESENT_REQUEST_OUT_OF_RANGE, 0); + *res->presentStatus = Z_PresentStatus_failure; + return; + } + Odr_int start = *req->resultSetStartPoint; + if (start < 1 || start > INT_MAX - toget) + { + res->records = create_nonSurrogateDiagnostics(odr_encode(), + YAZ_BIB1_PRESENT_REQUEST_OUT_OF_RANGE, 0); + *res->presentStatus = Z_PresentStatus_failure; + return; + } res->records = - pack_records (s, req->resultSetId,*req->resultSetStartPoint, - *req->numberOfRecordsRequested, - req->recordComposition, - res->nextResultSetPosition, - res->presentStatus, - req->preferredRecordSyntax); + pack_records(s, req->resultSetId, + (int) start, + (int) toget, + req->recordComposition, + res->nextResultSetPosition, + res->presentStatus, + req->preferredRecordSyntax); if (res->records->which == Z_Records_DBOSD) *res->numberOfRecordsReturned = res->records->u.databaseOrSurDiagnostics->num_records; @@ -217,8 +244,8 @@ int Yaz_Facility_Retrieval::recv(Z_Server *s, Z_APDU *apdu_request) case Z_APDU_searchRequest: apdu_response = s->create_Z_PDU(Z_APDU_searchResponse); s->transfer_referenceId(apdu_request, apdu_response); - sr_search (apdu_request->u.searchRequest, - apdu_response->u.searchResponse); + sr_search(apdu_request->u.searchRequest, + apdu_response->u.searchResponse); if (!apdu_response->u.searchResponse->records) { fetch_via_piggyback(s, apdu_request->u.searchRequest, @@ -228,12 +255,12 @@ int Yaz_Facility_Retrieval::recv(Z_Server *s, Z_APDU *apdu_request) return 1; case Z_APDU_presentRequest: apdu_response = s->create_Z_PDU(Z_APDU_presentResponse); + Z_PresentRequest *req = apdu_request->u.presentRequest; s->transfer_referenceId(apdu_request, apdu_response); - sr_present (apdu_request->u.presentRequest, - apdu_response->u.presentResponse); + sr_present(req, apdu_response->u.presentResponse); if (!apdu_response->u.presentResponse->records) fetch_via_present(s, apdu_request->u.presentRequest, - apdu_response->u.presentResponse); + apdu_response->u.presentResponse); s->send_Z_PDU(apdu_response, 0); return 1; }