Skip to content

Commit

Permalink
Rework on CORE-5277 to avoid CORE-5304 regression.
Browse files Browse the repository at this point in the history
CORE-5277 - Parameters with multibyte character sets allow to bypass the character limit of varchar fields.
CORE-5304 - Regression: Can not restore database with table contains field CHAR(n) and UTF8 character set.
  • Loading branch information
asfernandes committed Jul 13, 2016
1 parent aaeff69 commit 0d46c91
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 48 deletions.
49 changes: 49 additions & 0 deletions src/dsql/ExprNodes.cpp
Expand Up @@ -7922,6 +7922,55 @@ dsc* ParameterNode::execute(thread_db* tdbb, jrd_req* request) const

if (!(*impure_flags & VLU_checked))
{
if (!(request->req_flags & req_null))
{
desc = &impure->vlu_desc;

if (DTYPE_IS_TEXT(desc->dsc_dtype))
{
const UCHAR* p = desc->dsc_address;
USHORT maxLen = desc->dsc_length;
USHORT len;

switch (desc->dsc_dtype)
{
case dtype_cstring:
len = strlen((const char*) p);
--maxLen;
break;

case dtype_text:
len = desc->dsc_length;
break;

case dtype_varying:
len = reinterpret_cast<const vary*>(p)->vary_length;
p += sizeof(USHORT);
maxLen -= sizeof(USHORT);
break;
}

CharSet* charSet = INTL_charset_lookup(tdbb, DSC_GET_CHARSET(desc));

EngineCallbacks::instance->validateData(charSet, len, p);
EngineCallbacks::instance->validateLength(charSet, len, p, maxLen);
}
else if (desc->isBlob())
{
if (desc->getCharSet() != CS_NONE && desc->getCharSet() != CS_BINARY)
{
const Jrd::bid* bid = request->getImpure<Jrd::bid>(
message->impureOffset + (ULONG)(IPTR) desc->dsc_address);

if (!bid->isEmpty())
{
AutoBlb blob(tdbb, blb::open(tdbb, tdbb->getTransaction(), bid));
blob.getBlb()->BLB_check_well_formed(tdbb, desc);
}
}
}
}

if (argInfo)
{
EVL_validate(tdbb, Item(Item::TYPE_PARAMETER, message->messageNumber, argNumber),
Expand Down
8 changes: 4 additions & 4 deletions src/jrd/cvt.cpp
Expand Up @@ -419,17 +419,17 @@ void EngineCallbacks::validateData(CharSet* toCharSet, SLONG length, const UCHAR
void EngineCallbacks::validateLength(CharSet* toCharSet, SLONG toLength, const UCHAR* start,
const USHORT to_size)
{
if (toCharSet)
if (toCharSet &&
toCharSet->isMultiByte() &&
!(toCharSet->getFlags() & CHARSET_LEGACY_SEMANTICS))
{
Jrd::thread_db* tdbb = NULL;
SET_TDBB(tdbb);

const ULONG src_len = toCharSet->length(toLength, start, false);
const ULONG dest_len = (ULONG) to_size / toCharSet->maxBytesPerChar();

if (toCharSet->isMultiByte() &&
!(toCharSet->getFlags() & CHARSET_LEGACY_SEMANTICS) &&
src_len > dest_len)
if (src_len > dest_len)
{
err(Arg::Gds(isc_arith_except) << Arg::Gds(isc_string_truncation) <<
Arg::Gds(isc_trunc_limits) << Arg::Num(dest_len) << Arg::Num(src_len));
Expand Down
44 changes: 0 additions & 44 deletions src/jrd/exe.cpp
Expand Up @@ -786,50 +786,6 @@ void EXE_send(thread_db* tdbb, jrd_req* request, USHORT msg, ULONG length, const

memcpy(request->getImpure<UCHAR>(message->impureOffset), buffer, length);

for (USHORT i = 0; i < format->fmt_count; ++i)
{
const DSC* desc = &format->fmt_desc[i];

// ASF: I'll not test for dtype_cstring because usage is only internal
if (desc->dsc_dtype == dtype_text || desc->dsc_dtype == dtype_varying)
{
const UCHAR* p = request->getImpure<UCHAR>(message->impureOffset +
(ULONG)(IPTR) desc->dsc_address);
USHORT len;

switch (desc->dsc_dtype)
{
case dtype_text:
len = desc->dsc_length;
break;

case dtype_varying:
len = reinterpret_cast<const vary*>(p)->vary_length;
p += sizeof(USHORT);
break;
}

CharSet* charSet = INTL_charset_lookup(tdbb, DSC_GET_CHARSET(desc));

if (!charSet->wellFormed(len, p))
ERR_post(Arg::Gds(isc_malformed_string));
}
else if (desc->isBlob())
{
if (desc->getCharSet() != CS_NONE && desc->getCharSet() != CS_BINARY)
{
const Jrd::bid* bid = request->getImpure<Jrd::bid>(
message->impureOffset + (ULONG)(IPTR) desc->dsc_address);

if (!bid->isEmpty())
{
AutoBlb blob(tdbb, blb::open(tdbb, transaction/*tdbb->getTransaction()*/, bid));
blob.getBlb()->BLB_check_well_formed(tdbb, desc);
}
}
}
}

execute_looper(tdbb, request, transaction, request->req_next, jrd_req::req_proceed);
}

Expand Down

3 comments on commit 0d46c91

@dyemanov
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit fixed some problems with restore but introduced new ones, see the QA results. Prior build (32556) didn't have such failures.

@asfernandes
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verifying...

@asfernandes
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hope the new commit fixes all the problems now.

Please sign in to comment.