Some of icompare overloads are error prone

[teksturi]

Describe the bug

RemoteSyslogChannel "LOG_" and "SYSLOG_" properties cannot work. (Not tested looking through code.)

We have following code.

void RemoteSyslogChannel::setProperty(const std::string& name, const std::string& value)
{
    ...
   
		std::string facility;
		if (Poco::icompare(value, 4, "LOG_") == 0)
			facility = Poco::toUpper(value.substr(4));
		else if (Poco::icompare(value, 7, "SYSLOG_") == 0)
			facility = Poco::toUpper(value.substr(7));
		else
			facility = Poco::toUpper(value);

Looks valid. But there is catch. icompare has following overloads

int Foundation_API icompare(const std::string& str, std::string::size_type pos, const std::string::value_type* ptr);
int Foundation_API icompare(const std::string& str1, std::string::size_type n, const std::string& str2);

So second parameter is pos and not n. So basically this has never worked? This is only place where this pointer icompare overload is actually used. So what to do to fix this. do we change pos to n or remove deprecate that function first and remove it later? I can of course just fix RemoteSyslogChannel and leave this but would love to fix problem which leads to this.

Please add relevant environment information:

• Bug seems to be in over 13 years.

[teksturi]

Ok I also found something else strange of icompare.

This does not return 0 with size of 0.

template <class S>
int icompare(
	const S& str,
	typename S::size_type pos,
	typename S::size_type n,
	const typename S::value_type* ptr)
{
        // I would put zero check here before ptr check as it is valid even with nullptr if size is 0

	poco_check_ptr (ptr);
	typename S::size_type sz = str.size();
	if (pos > sz) pos = sz;
	if (pos + n > sz) n = sz - pos;
	typename S::const_iterator it  = str.begin() + pos;
	typename S::const_iterator end = str.begin() + pos + n;
	while (it != end && *ptr)
	{
		typename S::value_type c1(static_cast<typename S::value_type>(Ascii::toLower(*it)));
		typename S::value_type c2(static_cast<typename S::value_type>(Ascii::toLower(*ptr)));
		if (c1 < c2)
			return -1;
		else if (c1 > c2)
			return 1;
		++it; ++ptr;
	}

	if (it == end)
		return *ptr == 0 ? 0 : -1;
	else
		return 1;
}

We have even unittests for this

void StringTest::testIcompare()
{
    ....
    std::string s4 = "cCcCc";
    std::string s5;
   
    // This is correct but internally it use next icompare prototype.
    assertTrue (icompare(s5, s4.c_str()) < 0);
}

template <class S>
int icompare(
	const S& str,
	const typename S::value_type* ptr)
{
        // This is wrong imo. With
	return icompare(str, 0, str.size(), ptr);
}

I would have guessed that this works same way as string compare or strncmp. See https://godbolt.org/z/vqrGjxKn7

Basically this issue seems to evolve icompare discussion. There is so many ways to fix this issue so I would like little conversion what you guys want. We probably can agree that current interfaces are not right with icompare.

[github-actions[bot]]

This issue is stale because it has been open for 365 days with no activity.