From 4fbf4ee03dab01fda21f00851954bedc8dbc515a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Valentin=20Gr=C3=BCnbacher?= Date: Sun, 15 Mar 2015 22:38:49 +0100 Subject: [PATCH 1/6] Merge datapack updates from SoureMod --- amxmodx/CDataPack.cpp | 56 ++++++++++++++++++++++++++++++++++--------- amxmodx/CDataPack.h | 11 +++++++-- amxmodx/datapacks.cpp | 4 ++-- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/amxmodx/CDataPack.cpp b/amxmodx/CDataPack.cpp index 658e8484..417bf69d 100644 --- a/amxmodx/CDataPack.cpp +++ b/amxmodx/CDataPack.cpp @@ -8,7 +8,7 @@ * This program is free software; you can redistribute it and/or modify it under * the terms of the GNU General Public License, version 3.0, as published by the * Free Software Foundation. - * + * * This program is distributed in the hope that it will be useful, but WITHOUT * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more @@ -74,9 +74,12 @@ void CDataPack::ResetSize() size_t CDataPack::CreateMemory(size_t size, void **addr) { - CheckSize(sizeof(size_t) + size); + CheckSize(sizeof(char) + sizeof(size_t) + size); size_t pos = m_curptr - m_pBase; + *(char *)m_curptr = Raw; + m_curptr += sizeof(char); + *(size_t *)m_curptr = size; m_curptr += sizeof(size_t); @@ -86,14 +89,17 @@ size_t CDataPack::CreateMemory(size_t size, void **addr) } m_curptr += size; - m_size += sizeof(size_t) + size; + m_size += sizeof(char) + sizeof(size_t) + size; return pos; } void CDataPack::PackCell(cell cells) { - CheckSize(sizeof(size_t) + sizeof(cell)); + CheckSize(sizeof(char) + sizeof(size_t) + sizeof(cell)); + + *(char *)m_curptr = DataPackType::Cell; + m_curptr += sizeof(char); *(size_t *)m_curptr = sizeof(cell); m_curptr += sizeof(size_t); @@ -101,12 +107,15 @@ void CDataPack::PackCell(cell cells) *(cell *)m_curptr = cells; m_curptr += sizeof(cell); - m_size += sizeof(size_t) + sizeof(cell); + m_size += sizeof(char) + sizeof(size_t) + sizeof(cell); } void CDataPack::PackFloat(float val) { - CheckSize(sizeof(size_t) + sizeof(float)); + CheckSize(sizeof(char) + sizeof(size_t) + sizeof(float)); + + *(char *)m_curptr = DataPackType::Float; + m_curptr += sizeof(char); *(size_t *)m_curptr = sizeof(float); m_curptr += sizeof(size_t); @@ -114,15 +123,18 @@ void CDataPack::PackFloat(float val) *(float *)m_curptr = val; m_curptr += sizeof(float); - m_size += sizeof(size_t) + sizeof(float); + m_size += sizeof(char) + sizeof(size_t) + sizeof(float); } void CDataPack::PackString(const char *string) { size_t len = strlen(string); - size_t maxsize = sizeof(size_t) + len + 1; + size_t maxsize = sizeof(char) + sizeof(size_t) + len + 1; CheckSize(maxsize); + *(char *)m_curptr = DataPackType::String; + m_curptr += sizeof(char); + // Pack the string length first for buffer overrun checking. *(size_t *)m_curptr = len; m_curptr += sizeof(size_t); @@ -158,10 +170,16 @@ bool CDataPack::SetPosition(size_t pos) const cell CDataPack::ReadCell() const { - if (!IsReadable(sizeof(size_t) + sizeof(cell))) + if (!IsReadable(sizeof(char) + sizeof(size_t) + sizeof(cell))) { return 0; } + if (*reinterpret_cast(m_curptr) != DataPackType::Cell) + { + return 0; + } + m_curptr += sizeof(char); + if (*reinterpret_cast(m_curptr) != sizeof(cell)) { return 0; @@ -176,10 +194,16 @@ cell CDataPack::ReadCell() const float CDataPack::ReadFloat() const { - if (!IsReadable(sizeof(size_t) + sizeof(float))) + if (!IsReadable(sizeof(char) + sizeof(size_t) + sizeof(float))) { return 0; } + if (*reinterpret_cast(m_curptr) != DataPackType::Float) + { + return 0; + } + m_curptr += sizeof(char); + if (*reinterpret_cast(m_curptr) != sizeof(float)) { return 0; @@ -199,10 +223,15 @@ bool CDataPack::IsReadable(size_t bytes) const const char *CDataPack::ReadString(size_t *len) const { - if (!IsReadable(sizeof(size_t))) + if (!IsReadable(sizeof(char) + sizeof(size_t))) { return NULL; } + if (*reinterpret_cast(m_curptr) != DataPackType::String) + { + return NULL; + } + m_curptr += sizeof(char); size_t real_len = *(size_t *)m_curptr; @@ -235,6 +264,11 @@ void *CDataPack::ReadMemory(size_t *size) const { return NULL; } + if (*reinterpret_cast(m_curptr) != DataPackType::Raw) + { + return NULL; + } + m_curptr += sizeof(char); size_t bytecount = *(size_t *)m_curptr; m_curptr += sizeof(size_t); diff --git a/amxmodx/CDataPack.h b/amxmodx/CDataPack.h index 618680d7..2908ee06 100644 --- a/amxmodx/CDataPack.h +++ b/amxmodx/CDataPack.h @@ -8,7 +8,7 @@ * This program is free software; you can redistribute it and/or modify it under * the terms of the GNU General Public License, version 3.0, as published by the * Free Software Foundation. - * + * * This program is distributed in the hope that it will be useful, but WITHOUT * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more @@ -79,7 +79,7 @@ public: /** * @brief Returns whether or not a specified number of bytes from the current stream * position to the end can be read. - * + * * @param bytes Number of bytes to simulate reading. * @return True if can be read, false otherwise. */ @@ -160,6 +160,13 @@ private: mutable char *m_curptr; size_t m_capacity; size_t m_size; + + enum DataPackType { + Raw, + Cell, + Float, + String, + }; }; class CDataPackHandles diff --git a/amxmodx/datapacks.cpp b/amxmodx/datapacks.cpp index 82a82034..c886ae02 100644 --- a/amxmodx/datapacks.cpp +++ b/amxmodx/datapacks.cpp @@ -94,7 +94,7 @@ static cell AMX_NATIVE_CALL ReadPackCell(AMX* amx, cell* params) return 0; } - if (!d->IsReadable(sizeof(size_t) + sizeof(cell))) + if (!d->IsReadable(sizeof(char) + sizeof(size_t) + sizeof(cell))) { LogError(amx, AMX_ERR_NATIVE, "DataPack operation is out of bounds."); return 0; @@ -113,7 +113,7 @@ static cell AMX_NATIVE_CALL ReadPackFloat(AMX* amx, cell* params) return 0; } - if (!d->IsReadable(sizeof(size_t) + sizeof(float))) + if (!d->IsReadable(sizeof(char) + sizeof(size_t) + sizeof(float))) { LogError(amx, AMX_ERR_NATIVE, "DataPack operation is out of bounds."); return 0; From 9d4c02f7cad72e3d59638515f32e11ee53741457 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Valentin=20Gr=C3=BCnbacher?= Date: Sun, 15 Mar 2015 22:56:31 +0100 Subject: [PATCH 2/6] Nuke IsPackReadable This native exposes/requires knowledge about implementation details of the internal DataPack structure. Plugins can not properly deal with this, and abusing this functionality results in a chance of future breakage. --- amxmodx/datapacks.cpp | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/amxmodx/datapacks.cpp b/amxmodx/datapacks.cpp index c886ae02..7e1de2e6 100644 --- a/amxmodx/datapacks.cpp +++ b/amxmodx/datapacks.cpp @@ -197,19 +197,6 @@ static cell AMX_NATIVE_CALL SetPackPosition(AMX* amx, cell* params) return 1; } -static cell AMX_NATIVE_CALL IsPackReadable(AMX* amx, cell* params) -{ - CDataPack *d = g_DataPackHandles.lookup(params[1]); - - if (d == NULL) - { - LogError(amx, AMX_ERR_NATIVE, "Invalid datapack handle provided (%d)", params[1]); - return 0; - } - - return d->IsReadable(params[2]) ? 1 : 0; -} - static cell AMX_NATIVE_CALL DestroyDataPack(AMX* amx, cell* params) { cell *ptr = get_amxaddr(amx, params[1]); @@ -243,7 +230,6 @@ AMX_NATIVE_INFO g_DatapackNatives[] = { "ResetPack", ResetPack }, { "GetPackPosition", GetPackPosition }, { "SetPackPosition", SetPackPosition }, - { "IsPackReadable", IsPackReadable }, { "DestroyDataPack", DestroyDataPack }, {NULL, NULL} }; From 9abe6cd8f6ab9c7447f00a887785cdd63948fa34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Valentin=20Gr=C3=BCnbacher?= Date: Mon, 30 Mar 2015 03:56:26 +0200 Subject: [PATCH 3/6] Add IsPackEnded native (IsPackReadable replacement) --- amxmodx/datapacks.cpp | 14 ++++++++++++++ plugins/include/datapack.inc | 11 +++++++++++ 2 files changed, 25 insertions(+) diff --git a/amxmodx/datapacks.cpp b/amxmodx/datapacks.cpp index 7e1de2e6..a61ec8d1 100644 --- a/amxmodx/datapacks.cpp +++ b/amxmodx/datapacks.cpp @@ -197,6 +197,19 @@ static cell AMX_NATIVE_CALL SetPackPosition(AMX* amx, cell* params) return 1; } +static cell AMX_NATIVE_CALL IsPackEnded(AMX* amx, cell* params) +{ + CDataPack *d = g_DataPackHandles.lookup(params[1]); + + if (d == NULL) + { + LogError(amx, AMX_ERR_NATIVE, "Invalid datapack handle provided (%d)", params[1]); + return 0; + } + + return d->IsReadable(1) ? false : true; +} + static cell AMX_NATIVE_CALL DestroyDataPack(AMX* amx, cell* params) { cell *ptr = get_amxaddr(amx, params[1]); @@ -230,6 +243,7 @@ AMX_NATIVE_INFO g_DatapackNatives[] = { "ResetPack", ResetPack }, { "GetPackPosition", GetPackPosition }, { "SetPackPosition", SetPackPosition }, + { "IsPackEnded", IsPackEnded }, { "DestroyDataPack", DestroyDataPack }, {NULL, NULL} }; diff --git a/plugins/include/datapack.inc b/plugins/include/datapack.inc index fd2743b3..1b6894fd 100644 --- a/plugins/include/datapack.inc +++ b/plugins/include/datapack.inc @@ -141,6 +141,17 @@ native GetPackPosition(DataPack:pack); */ native SetPackPosition(DataPack:pack, position); +/** + * Returns if the datapack has reached its end and no more data can be read. + * + * @param pack Datapack handle + * + * @return True if datapack has reached the end, false otherwise + * @error If an invalid handle is provided, or the new position is + * out of datapack bounds, an error will be thrown. + */ + native bool:IsPackEnded(DataPack:pack); + /** * Destroys the datapack and frees its memory. * From 1d57677426fe90cbeadd24afaed21424b4635fb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Valentin=20Gr=C3=BCnbacher?= Date: Sun, 15 Mar 2015 22:58:14 +0100 Subject: [PATCH 4/6] Add CanRead[Cell|Float|String|Memory] to CDataPack --- amxmodx/CDataPack.cpp | 114 ++++++++++++++++++++++++++++++------------ amxmodx/CDataPack.h | 5 ++ 2 files changed, 87 insertions(+), 32 deletions(-) diff --git a/amxmodx/CDataPack.cpp b/amxmodx/CDataPack.cpp index 417bf69d..790faa3a 100644 --- a/amxmodx/CDataPack.cpp +++ b/amxmodx/CDataPack.cpp @@ -168,23 +168,32 @@ bool CDataPack::SetPosition(size_t pos) const return true; } -cell CDataPack::ReadCell() const +bool CDataPack::CanReadCell() const { if (!IsReadable(sizeof(char) + sizeof(size_t) + sizeof(cell))) { - return 0; + return false; } if (*reinterpret_cast(m_curptr) != DataPackType::Cell) { - return 0; + return false; + } + if (*reinterpret_cast(m_curptr + sizeof(char)) != sizeof(cell)) + { + return false; } - m_curptr += sizeof(char); - if (*reinterpret_cast(m_curptr) != sizeof(cell)) + return true; +} + +cell CDataPack::ReadCell() const +{ + if (!CanReadCell()) { return 0; } + m_curptr += sizeof(char); m_curptr += sizeof(size_t); cell val = *reinterpret_cast(m_curptr); @@ -192,23 +201,32 @@ cell CDataPack::ReadCell() const return val; } -float CDataPack::ReadFloat() const +bool CDataPack::CanReadFloat() const { if (!IsReadable(sizeof(char) + sizeof(size_t) + sizeof(float))) { - return 0; + return false; } if (*reinterpret_cast(m_curptr) != DataPackType::Float) { - return 0; + return false; + } + if (*reinterpret_cast(m_curptr + sizeof(char)) != sizeof(float)) + { + return false; } - m_curptr += sizeof(char); - if (*reinterpret_cast(m_curptr) != sizeof(float)) + return true; +} + +float CDataPack::ReadFloat() const +{ + if (!CanReadFloat()) { return 0; } + m_curptr += sizeof(char); m_curptr += sizeof(size_t); float val = *reinterpret_cast(m_curptr); @@ -221,26 +239,23 @@ bool CDataPack::IsReadable(size_t bytes) const return (bytes + (m_curptr - m_pBase) > m_size) ? false : true; } -const char *CDataPack::ReadString(size_t *len) const +bool CDataPack::CanReadString(size_t *len) const { if (!IsReadable(sizeof(char) + sizeof(size_t))) { - return NULL; + return false; } if (*reinterpret_cast(m_curptr) != DataPackType::String) { - return NULL; + return false; } - m_curptr += sizeof(char); - size_t real_len = *(size_t *)m_curptr; + size_t real_len = *(size_t *)(m_curptr + sizeof(char)); + char *str = (char *)(m_curptr + sizeof(char) + sizeof(size_t)); - m_curptr += sizeof(size_t); - char *str = (char *)m_curptr; - - if ((strlen(str) != real_len) || !(IsReadable(real_len+1))) + if ((strlen(str) != real_len) || !(IsReadable(sizeof(char) + sizeof(size_t) + real_len + 1))) { - return NULL; + return false; } if (len) @@ -248,8 +263,28 @@ const char *CDataPack::ReadString(size_t *len) const *len = real_len; } + return true; +} + +const char *CDataPack::ReadString(size_t *len) const +{ + size_t real_len; + if (!CanReadString(&real_len)) + { + return NULL; + } + + m_curptr += sizeof(char); + m_curptr += sizeof(size_t); + + char *str = (char *)m_curptr; m_curptr += real_len + 1; + if (len) + { + *len = real_len; + } + return str; } @@ -258,34 +293,49 @@ void *CDataPack::GetMemory() const return m_curptr; } -void *CDataPack::ReadMemory(size_t *size) const +bool CDataPack::CanReadMemory(size_t *size) const { - if (!IsReadable(sizeof(size_t))) + if (!IsReadable(sizeof(char) + sizeof(size_t))) { - return NULL; + return false; } if (*reinterpret_cast(m_curptr) != DataPackType::Raw) { - return NULL; + return false; } - m_curptr += sizeof(char); - size_t bytecount = *(size_t *)m_curptr; - m_curptr += sizeof(size_t); - - if (!IsReadable(bytecount)) + size_t bytecount = *(size_t *)(m_curptr + sizeof(char)); + if (!IsReadable(sizeof(char) + sizeof(size_t) + bytecount)) { - return NULL; + return false; } - void *ptr = m_curptr; - if (size) { *size = bytecount; } + return true; +} + +void *CDataPack::ReadMemory(size_t *size) const +{ + size_t bytecount; + if (!CanReadMemory(&bytecount)) + { + return NULL; + } + + m_curptr += sizeof(char); + m_curptr += sizeof(size_t); + + void *ptr = m_curptr; m_curptr += bytecount; + if (size) + { + *size = bytecount; + } + return ptr; } diff --git a/amxmodx/CDataPack.h b/amxmodx/CDataPack.h index 2908ee06..9c91cea2 100644 --- a/amxmodx/CDataPack.h +++ b/amxmodx/CDataPack.h @@ -108,6 +108,11 @@ public: */ void *ReadMemory(size_t *size) const; + bool CanReadCell() const; + bool CanReadFloat() const; + bool CanReadString(size_t *len) const; + bool CanReadMemory(size_t *size) const; + public: /** * @brief Resets the used size of the stream back to zero. From 2166c320470f5a545c58c8114e961a473900b154 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Valentin=20Gr=C3=BCnbacher?= Date: Sat, 28 Mar 2015 14:04:50 +0100 Subject: [PATCH 5/6] Make datapack natives error consistently, stop using IsPackReadable --- amxmodx/datapacks.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/amxmodx/datapacks.cpp b/amxmodx/datapacks.cpp index a61ec8d1..7da0c0ff 100644 --- a/amxmodx/datapacks.cpp +++ b/amxmodx/datapacks.cpp @@ -94,9 +94,9 @@ static cell AMX_NATIVE_CALL ReadPackCell(AMX* amx, cell* params) return 0; } - if (!d->IsReadable(sizeof(char) + sizeof(size_t) + sizeof(cell))) + if (!d->CanReadCell()) { - LogError(amx, AMX_ERR_NATIVE, "DataPack operation is out of bounds."); + LogError(amx, AMX_ERR_NATIVE, "Datapack operation is invalid."); return 0; } @@ -113,9 +113,9 @@ static cell AMX_NATIVE_CALL ReadPackFloat(AMX* amx, cell* params) return 0; } - if (!d->IsReadable(sizeof(char) + sizeof(size_t) + sizeof(float))) + if (!d->CanReadFloat()) { - LogError(amx, AMX_ERR_NATIVE, "DataPack operation is out of bounds."); + LogError(amx, AMX_ERR_NATIVE, "Datapack operation is invalid."); return 0; } @@ -134,14 +134,15 @@ static cell AMX_NATIVE_CALL ReadPackString(AMX* amx, cell* params) return 0; } - const char *str; - size_t len; - if (!(str = d->ReadString(&len))) + if (!d->CanReadString(NULL)) { - LogError(amx, AMX_ERR_NATIVE, "DataPack operation is out of bounds."); + LogError(amx, AMX_ERR_NATIVE, "Datapack operation is invalid."); return 0; } + size_t len; + const char *str = d->ReadString(&len); + return set_amxstring_utf8(amx, params[2], str, len, params[3] + 1); // + EOS } From 081b683e03c4203a8c12d3bcc224d504032c7bee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Valentin=20Gr=C3=BCnbacher?= Date: Mon, 16 Mar 2015 00:28:40 +0100 Subject: [PATCH 6/6] Update datapack test plugin --- plugins/testsuite/datapack_test.sma | 68 ++++++++++++++++------------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/plugins/testsuite/datapack_test.sma b/plugins/testsuite/datapack_test.sma index bba11d63..c076e8ff 100644 --- a/plugins/testsuite/datapack_test.sma +++ b/plugins/testsuite/datapack_test.sma @@ -41,51 +41,57 @@ public datapacktest() { failcount = 0; passcount = 0; - + new DataPack:pack = CreateDataPack(); new DataPack:oldPack = pack; // Makes sure that the trie handle system recycles old handles - + new refCell = 23; new Float:refFloat = 42.42; new refString[] = "I'm a little teapot."; - + // Write - WritePackCell(pack, refCell); // 8 - WritePackString(pack, refString); // 25 (sizeof string + 4) - WritePackFloat(pack, refFloat); // 8 - - test("Position #1 test", .pass = GetPackPosition(pack) == 41); - test("Readable #1 test", .pass = !IsPackReadable(pack, 41)); - + new cellPos = GetPackPosition(pack); + WritePackCell(pack, refCell); + new floatPos = GetPackPosition(pack); + WritePackFloat(pack, refFloat); + new strPos = GetPackPosition(pack); + WritePackString(pack, refString); + new endPos = GetPackPosition(pack); + + test("Write position test", + .pass = (cellPos != floatPos && cellPos != strPos && cellPos != endPos + && floatPos != strPos && floatPos != endPos && strPos != endPos)); + //resets the index to the beginning, necessary for read. ResetPack(pack); - - test("Position #2 test", .pass = GetPackPosition(pack) == 0 ); - test("Readable #2 test", .pass = IsPackReadable(pack, 15)); - - // Read + + test("Position #1 test", .pass = (GetPackPosition(pack) == cellPos)); + test("Readable #1 test", .pass = !IsPackEnded(pack)); + new cellValue = ReadPackCell(pack); + test("Cell test", .pass = (cellValue == refCell)); + + test("Position #2 test", .pass = (GetPackPosition(pack) == floatPos)); + test("Readable #2 test", .pass = !IsPackEnded(pack)); + + new Float:floatValue = ReadPackFloat(pack); + test("Float test", .pass = (floatValue == refFloat)); + + test("Position #3 test", .pass = (GetPackPosition(pack) == strPos)); + test("Readable #3 test", .pass = !IsPackEnded(pack)); + new buffer[1024]; ReadPackString(pack, buffer, 1024); - new Float:floatvalue = ReadPackFloat(pack); - - test("Cell test", .pass = cellValue == refCell); - test("String test", .pass = bool:equal(buffer, refString)); - test("Float test #1", .pass = floatvalue == refFloat); - - SetPackPosition(pack, 33); - test("Set Position test", .pass = GetPackPosition(pack) == 33); - - WritePackFloat(pack, refFloat + 1); - SetPackPosition(pack, 33); - test("Float test #2", .pass = ReadPackFloat(pack) == refFloat + 1); + test("String test #1", .pass = bool:equal(buffer, refString)); + + test("End test", .pass = IsPackEnded(pack)); ResetPack(pack, .clear = true); - test("Clear test", .pass = !IsPackReadable(pack, 15)); - + test("Clear test", .pass = IsPackEnded(pack)); + DestroyDataPack(pack); - + test("Recycle handles", CreateDataPack() == oldPack); - + done(); }