Skip to content

Fix API inconsistency of NULL Date #136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 15, 2024
Merged

Fix API inconsistency of NULL Date #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb50db8
Add test to write NULL Date of any width and check for NULL
thbeu Jul 15, 2024
c116845
Accept string containing of width times "0" as NULL Date
thbeu Jul 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions dbfopen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1119,7 +1119,7 @@ SHPDate SHPAPI_CALL DBFReadDateAttribute(DBFHandle psDBF, int iRecord,
/* Return TRUE if the passed string is NULL. */
/************************************************************************/

static bool DBFIsValueNULL(char chType, const char *pszValue)
static bool DBFIsValueNULL(char chType, const char *pszValue, int size)
{
if (pszValue == SHPLIB_NULLPTR)
return true;
Expand All @@ -1144,14 +1144,19 @@ static bool DBFIsValueNULL(char chType, const char *pszValue)
return true;

case 'D':
/* NULL date fields have value "00000000" */
/* NULL date fields have value "00000000" or "0"*size */
/* Some DBF files have fields filled with spaces */
/* (trimmed by DBFReadStringAttribute) to indicate null */
/* values for dates (#4265). */
/* And others have ' 0': https://lists.osgeo.org/pipermail/gdal-dev/2023-November/058010.html */
/* And others just empty string: https://github.com/OSGeo/gdal/issues/10405 */
return pszValue[0] == 0 || strncmp(pszValue, "00000000", 8) == 0 ||
strcmp(pszValue, " ") == 0 || strcmp(pszValue, "0") == 0;
if (pszValue[0] == 0 || strncmp(pszValue, "00000000", 8) == 0 ||
strcmp(pszValue, " ") == 0 || strcmp(pszValue, "0") == 0)
return true;
for (int i = 0; i < size; i++)
if (pszValue[i] != '0')
return false;
return true;

case 'L':
/* NULL boolean fields have value "?" */
Expand Down Expand Up @@ -1179,7 +1184,8 @@ int SHPAPI_CALL DBFIsAttributeNULL(const DBFHandle psDBF, int iRecord,
if (pszValue == SHPLIB_NULLPTR)
return TRUE;

return DBFIsValueNULL(psDBF->pachFieldType[iField], pszValue);
return DBFIsValueNULL(psDBF->pachFieldType[iField], pszValue,
psDBF->panFieldSize[iField]);
}

/************************************************************************/
Expand Down Expand Up @@ -2166,7 +2172,8 @@ int SHPAPI_CALL DBFAlterFieldDefn(DBFHandle psDBF, int iField,
}

memcpy(pszOldField, pszRecord + nOffset, nOldWidth);
const bool bIsNULL = DBFIsValueNULL(chOldType, pszOldField);
const bool bIsNULL =
DBFIsValueNULL(chOldType, pszOldField, nOldWidth);

if (nWidth != nOldWidth)
{
Expand Down Expand Up @@ -2243,7 +2250,8 @@ int SHPAPI_CALL DBFAlterFieldDefn(DBFHandle psDBF, int iField,
}

memcpy(pszOldField, pszRecord + nOffset, nOldWidth);
const bool bIsNULL = DBFIsValueNULL(chOldType, pszOldField);
const bool bIsNULL =
DBFIsValueNULL(chOldType, pszOldField, nOldWidth);

if (nOffset + nOldWidth < nOldRecordLength)
{
Expand Down
66 changes: 61 additions & 5 deletions tests/dbf_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ static auto WriteDate(const fs::path &filename,
return success;
}

static auto ReadDate(const fs::path &filename) -> auto
static auto ReadDate(const fs::path &filename, int expectIsNull,
int expectWidth) -> auto
{
const auto handle = DBFOpen(filename.string().c_str(), "rb");
EXPECT_NE(nullptr, handle);
Expand All @@ -147,8 +148,10 @@ static auto ReadDate(const fs::path &filename) -> auto
DBFGetFieldInfo(handle, 0, fieldname->data(), &width, &decimals);
EXPECT_EQ(FTDate, fieldtype);
EXPECT_EQ(0, std::strcmp("date", fieldname->data()));
EXPECT_EQ(8, width);
EXPECT_EQ(expectWidth, width);
EXPECT_EQ(0, decimals);
const auto isNull = DBFIsAttributeNULL(handle, 0, 0);
EXPECT_EQ(expectIsNull, isNull);
const auto recordcount = DBFGetRecordCount(handle);
EXPECT_EQ(1, recordcount);
const auto date = DBFReadDateAttribute(handle, 0, 0);
Expand Down Expand Up @@ -205,22 +208,75 @@ TEST(DBFFieldTest, SetAndGetDateToday)
EXPECT_TRUE(success);
const auto size = fs::file_size(filename);
EXPECT_EQ(75, size);
const auto date = ReadDate(filename);
const auto date = ReadDate(filename, 0, 8);
EXPECT_EQ(today->year, date->year);
EXPECT_EQ(today->month, date->month);
EXPECT_EQ(today->day, date->day);
fs::remove(filename);
}

TEST(DBFFieldTest, SetAndGetDateInvalid)
TEST(DBFFieldTest, SetAndGetDateEmpty)
{
const auto filename =
fs::temp_directory_path() / GenerateUniqueFilename(".dbf");
const auto success = WriteDate(filename, std::make_unique<const SHPDate>());
EXPECT_TRUE(success);
const auto size = fs::file_size(filename);
EXPECT_EQ(75, size);
const auto date = ReadDate(filename);
const auto date = ReadDate(filename, 1, 8);
EXPECT_EQ(0, date->year);
EXPECT_EQ(0, date->month);
EXPECT_EQ(0, date->day);
fs::remove(filename);
}

TEST(DBFFieldTest, SetAndGetDateNull)
{
const auto filename =
fs::temp_directory_path() / GenerateUniqueFilename(".dbf");
for (int width = 1; width <= XBASE_FLD_MAX_WIDTH; ++width)
{
const auto success = [&filename, &width]
{
const auto handle = DBFCreate(filename.string().c_str());
EXPECT_NE(nullptr, handle);
const auto fid = DBFAddField(handle, "date", FTDate, width, 0);
EXPECT_GE(fid, 0);
const auto success = DBFWriteNULLAttribute(handle, 0, 0);
DBFClose(handle);
return success;
}();
EXPECT_TRUE(success);
const auto size = fs::file_size(filename);
EXPECT_EQ(67 + width, size);
const auto date = ReadDate(filename, 1, width);
EXPECT_EQ(0, date->year);
EXPECT_EQ(0, date->month);
EXPECT_EQ(0, date->day);
fs::remove(filename);
}
}

TEST(DBFFieldTest, SetAndGetDateEmptyString)
{
const auto filename =
fs::temp_directory_path() / GenerateUniqueFilename(".dbf");
const auto success = [&filename]
{
const auto handle = DBFCreate(filename.string().c_str());
EXPECT_NE(nullptr, handle);
const auto fid = DBFAddField(handle, "date", FTDate, 8, 0);
EXPECT_GE(fid, 0);
constexpr const auto emptyString = "";
const auto success =
DBFWriteAttributeDirectly(handle, 0, 0, emptyString);
DBFClose(handle);
return success;
}();
EXPECT_TRUE(success);
const auto size = fs::file_size(filename);
EXPECT_EQ(75, size);
const auto date = ReadDate(filename, 1, 8);
EXPECT_EQ(0, date->year);
EXPECT_EQ(0, date->month);
EXPECT_EQ(0, date->day);
Expand Down