Merge pull request #8571 from aafemt/postfix8145

Postfixes for PR#8145

Author: dyemanov

src/common/cvt.cpp

@@ -1648,7 +1648,7 @@ double CVT_get_double(const dsc* desc, DecimalStatus decSt, ErrorFunction err, b
 }
 
 
-void CVT_move_common(const dsc* from, dsc* to, DecimalStatus decSt, Callbacks* cb)
+void CVT_move_common(const dsc* from, dsc* to, DecimalStatus decSt, Callbacks* cb, bool trustedSource)
 {
 /**************************************
  *
@@ -1669,7 +1669,13 @@ void CVT_move_common(const dsc* from, dsc* to, DecimalStatus decSt, Callbacks* c
 	// optimal, it would cost more to find the fast move than the
 	// fast move would gain.
 
-	if (DSC_EQUIV(from, to, false))
+	// But do not do it for strings because their length has not been validated until this moment
+	// (real source length must be validated against target maximum length
+	// and this is the first common place where both are present).
+
+	// ...unless these strings are coming from a trusted source (for example a cached record buffer)
+
+	if (DSC_EQUIV(from, to, false) && (trustedSource || !DTYPE_IS_TEXT(from->dsc_dtype)))
 	{
 		if (length) {
 			memcpy(p, q, length);
@@ -1983,6 +1989,9 @@ void CVT_move_common(const dsc* from, dsc* to, DecimalStatus decSt, Callbacks* c
 				if (cb->transliterate(from, to, charset2))
 					return;
 
+				// At this point both `from` and `to` are guaranteed to have the same charset and this is stored in charset2
+				// Because of this we can freely use `toCharset` against `from`.
+
 				{ // scope
 					USHORT strtype_unused;
 					UCHAR *ptr;
@@ -1993,8 +2002,23 @@ void CVT_move_common(const dsc* from, dsc* to, DecimalStatus decSt, Callbacks* c
 				const USHORT to_size = TEXT_LEN(to);
 				CharSet* toCharset = cb->getToCharset(charset2);
 
-				cb->validateData(toCharset, length, q);
-				ULONG toLength = cb->validateLength(toCharset, charset2, length, q, to_size);
+				ULONG toLength = length;
+
+				if (!trustedSource)
+				{
+					// Most likely data already has been validated once or twice, but another validation won't hurt much.
+					cb->validateData(toCharset, length, q);
+					toLength = cb->validateLength(toCharset, charset2, length, q, to_size);
+				}
+				else
+				{
+					// Silently truncate. In the wild this should never happen
+					if (length > to_size)
+					{
+						fb_assert(from->dsc_dtype == dtype_text);
+						toLength = to_size;
+					}
+				}
 
 				switch (to->dsc_dtype)
 				{
@@ -3777,7 +3801,7 @@ USHORT CVT_get_string_ptr(const dsc* desc, USHORT* ttype, UCHAR** address,
 }
 
 
-void CVT_move(const dsc* from, dsc* to, DecimalStatus decSt, ErrorFunction err)
+void CVT_move(const dsc* from, dsc* to, DecimalStatus decSt, ErrorFunction err, bool trustedSource)
 {
 /**************************************
  *
@@ -3790,5 +3814,5 @@ void CVT_move(const dsc* from, dsc* to, DecimalStatus decSt, ErrorFunction err)
  *
  **************************************/
 	CommonCallbacks callbacks(err);
-	CVT_move_common(from, to, decSt, &callbacks);
+	CVT_move_common(from, to, decSt, &callbacks, trustedSource);
 }

src/common/cvt.h

@@ -95,8 +95,8 @@ Firebird::Decimal128 CVT_get_dec128(const dsc*, Firebird::DecimalStatus, ErrorFu
 Firebird::Int128 CVT_get_int128(const dsc*, SSHORT, Firebird::DecimalStatus, ErrorFunction);
 Firebird::Int128 CVT_hex_to_int128(const char* str, USHORT len);
 USHORT CVT_make_string(const dsc*, USHORT, const char**, vary*, USHORT, Firebird::DecimalStatus, ErrorFunction);
-void CVT_move_common(const dsc*, dsc*, Firebird::DecimalStatus, Firebird::Callbacks*);
-void CVT_move(const dsc*, dsc*, Firebird::DecimalStatus, ErrorFunction);
+void CVT_move_common(const dsc*, dsc*, Firebird::DecimalStatus, Firebird::Callbacks*, bool trustedSource = false);
+void CVT_move(const dsc*, dsc*, Firebird::DecimalStatus, ErrorFunction, bool trustedSource = false);
 SSHORT CVT_decompose(const char*, USHORT, Firebird::Int128*, ErrorFunction);
 USHORT CVT_get_string_ptr(const dsc*, USHORT*, UCHAR**, vary*, USHORT, Firebird::DecimalStatus, ErrorFunction);
 USHORT CVT_get_string_ptr_common(const dsc*, USHORT*, UCHAR**, vary*, USHORT, Firebird::DecimalStatus, Firebird::Callbacks*);

src/dsql/ExprNodes.cpp

@@ -9982,6 +9982,19 @@ ParameterNode* ParameterNode::pass1(thread_db* tdbb, CompilerScratch* csb)
 			status_exception::raise(Arg::Gds(isc_ctxnotdef) << Arg::Gds(isc_random) << Arg::Str("Outer parameter has no outer scratch"));
 	}
 
+	const dsc& desc = format->fmt_desc[argNumber];
+	if (desc.isText())
+	{
+		// Remember expected maximum length in characters to be able to recognize format of the real data buffer later
+		const CharSet* charSet = INTL_charset_lookup(tdbb, desc.getCharSet());
+		USHORT length = TEXT_LEN(&desc);
+		if (charSet->isMultiByte())
+		{
+			length /= charSet->maxBytesPerChar();
+		}
+		maxCharLength = length;
+	}
+
 	return this;
 }
 
@@ -10045,9 +10058,6 @@ dsc* ParameterNode::execute(thread_db* tdbb, Request* request) const
 	{
 		if (impureForOuter)
 			EVL_make_value(tdbb, retDesc, impureForOuter);
-
-		if (retDesc->dsc_dtype == dtype_text)
-			INTL_adjust_text_descriptor(tdbb, retDesc);
 	}
 
 	auto impureFlags = paramRequest->getImpure<USHORT>(
@@ -10057,7 +10067,6 @@ dsc* ParameterNode::execute(thread_db* tdbb, Request* request) const
 	{
 		if (!(request->req_flags & req_null))
 		{
-			USHORT maxLen = desc->dsc_length;	// not adjusted length
 
 			if (DTYPE_IS_TEXT(retDesc->dsc_dtype))
 			{
@@ -10067,8 +10076,7 @@ dsc* ParameterNode::execute(thread_db* tdbb, Request* request) const
 				switch (retDesc->dsc_dtype)
 				{
 					case dtype_cstring:
-						len = static_cast<USHORT>(strnlen((const char*) p, maxLen));
-						--maxLen;
+						len = static_cast<USHORT>(strnlen((const char*) p, desc->dsc_length));
 						break;
 
 					case dtype_text:
@@ -10078,14 +10086,15 @@ dsc* ParameterNode::execute(thread_db* tdbb, Request* request) const
 					case dtype_varying:
 						len = reinterpret_cast<const vary*>(p)->vary_length;
 						p += sizeof(USHORT);
-						maxLen -= sizeof(USHORT);
 						break;
 				}
 
 				auto charSet = INTL_charset_lookup(tdbb, DSC_GET_CHARSET(retDesc));
 
 				EngineCallbacks::instance->validateData(charSet, len, p);
-				EngineCallbacks::instance->validateLength(charSet, DSC_GET_CHARSET(retDesc), len, p, maxLen);
+
+				// Validation of length for user-provided data against user-provided metadata makes a little sense here. Leave it to the real assignment.
+				// Besides in some cases overlong values are valid. For example `field like ?`
 			}
 			else if (retDesc->isBlob())
 			{
@@ -10114,6 +10123,25 @@ dsc* ParameterNode::execute(thread_db* tdbb, Request* request) const
 		*impureFlags |= VLU_checked;
 	}
 
+	// This block is after validation because having here a malformed data would produce a wrong result
+	if (!(request->req_flags & req_null) && retDesc->dsc_dtype == dtype_text && maxCharLength != 0)
+	{
+		// Data in the message buffer can be in a padded Firebird format or in an application-defined format with real length.
+		// API provides no way to distinguish these cases so we must use some heuristics:
+		// perform the adjustment only if the data length matches the length that would be expected in the padded format.
+
+		const CharSet* charSet = INTL_charset_lookup(tdbb, retDesc->getCharSet());
+
+		if (charSet->isMultiByte() && maxCharLength * charSet->maxBytesPerChar() == retDesc->dsc_length)
+		{
+			Firebird::HalfStaticArray<UCHAR, BUFFER_SMALL> buffer;
+
+			retDesc->dsc_length = charSet->substring(retDesc->dsc_length, retDesc->dsc_address,
+				retDesc->dsc_length, buffer.getBuffer(retDesc->dsc_length), 0,
+				maxCharLength);
+		}
+	}
+
 	return (request->req_flags & req_null) ? nullptr : retDesc;
 }
 

src/dsql/ExprNodes.h

@@ -1693,6 +1693,7 @@ class ParameterNode final : public TypedNode<ValueExprNode, ExprNode::TYPE_PARAM
 	// Message can be modified during merge of SP/view subtrees
 	USHORT messageNumber = MAX_USHORT;
 	USHORT argNumber = 0;
+	USHORT maxCharLength = 0;
 	bool outerDecl = false;
 };
 

src/gpre/cmp.cpp

@@ -1205,12 +1205,6 @@ static void cmp_procedure( gpre_req* request)
 	for (gpre_port* port = request->req_ports; port; port = port->por_next)
 		cmp_port(port, request);
 
-	if (request->req_values)
-	{
-		request->add_byte(blr_begin);
-		make_send(request->req_vport, request);
-	}
-
 	if (gpreGlob.sw_ids)
 	{
 		request->add_byte(blr_exec_pid);
@@ -1247,8 +1241,6 @@ static void cmp_procedure( gpre_req* request)
 	else
 		request->add_word(0);
 
-	if (request->req_values)
-		request->add_byte(blr_end);
 	request->add_byte(blr_end);
 	request->add_byte(blr_eoc);
 	request->req_length = request->req_blr - request->req_base;

src/jrd/cvt2.cpp

@@ -490,6 +490,17 @@ int CVT2_compare(const dsc* arg1, const dsc* arg2, Firebird::DecimalStatus decSt
 	switch (arg1->dsc_dtype)
 	{
 	case dtype_ex_timestamp_tz:
+		{
+			DSC desc;
+			MOVE_CLEAR(&desc, sizeof(desc));
+			desc.dsc_dtype = dtype_ex_timestamp_tz;
+			ISC_TIMESTAMP_TZ_EX datetime;
+			desc.dsc_length = sizeof(datetime);
+			desc.dsc_address = (UCHAR*) &datetime;
+			CVT_move(arg2, &desc, 0);
+			return CVT2_compare(arg1, &desc, 0);
+		}
+
 	case dtype_timestamp_tz:
 		{
 			DSC desc;
@@ -515,6 +526,17 @@ int CVT2_compare(const dsc* arg1, const dsc* arg2, Firebird::DecimalStatus decSt
 		}
 
 	case dtype_ex_time_tz:
+		{
+			DSC desc;
+			MOVE_CLEAR(&desc, sizeof(desc));
+			desc.dsc_dtype = dtype_ex_time_tz;
+			ISC_TIME_TZ_EX atime;
+			desc.dsc_length = sizeof(atime);
+			desc.dsc_address = (UCHAR*) &atime;
+			CVT_move(arg2, &desc, 0);
+			return CVT2_compare(arg1, &desc, 0);
+		}
+
 	case dtype_sql_time_tz:
 		{
 			DSC desc;

src/jrd/exe.cpp

@@ -462,6 +462,8 @@ void EXE_assignment(thread_db* tdbb, const ValueExprNode* to, dsc* from_desc, bo
 			}
 		}
 
+		// Strings will be validated in CVT_move()
+
 		if (DTYPE_IS_BLOB_OR_QUAD(from_desc->dsc_dtype) || DTYPE_IS_BLOB_OR_QUAD(to_desc->dsc_dtype))
 		{
 			// ASF: Don't let MOV_move call blb::move because MOV
@@ -493,6 +495,11 @@ void EXE_assignment(thread_db* tdbb, const ValueExprNode* to, dsc* from_desc, bo
 		{
 			MOV_move(tdbb, from_desc, to_desc);
 		}
+		else if (DTYPE_IS_TEXT(from_desc->dsc_dtype))
+		{
+			// Force slow move to properly handle the case when source string is provided with real length instead of padded length
+			MOV_move(tdbb, from_desc, to_desc);
+		}
 		else if (from_desc->dsc_dtype == dtype_short)
 		{
 			*((SSHORT*) to_desc->dsc_address) = *((SSHORT*) from_desc->dsc_address);

src/jrd/jrd.cpp

@@ -4152,6 +4152,26 @@ void JRequest::getInfo(CheckStatusWrapper* user_status, int level, unsigned int
 
 		try
 		{
+			for (unsigned i = 0; i < itemsLength; ++i)
+			{
+				if (items[i] == isc_info_message_number || items[i] == isc_info_message_size)
+				{
+					// For proper return these items require request operation req_send or req_receive
+					// Run request from stale status until we get one of these (or end of program)
+					// It is up to caller to make sure that there is no heavy operations between req_next
+					// and the next SuspendNode/ReceiveNode/SelectMessageNode
+					while ((request->req_flags & req_active)
+							&& request->req_operation != Request::req_receive
+							&& request->req_operation != Request::req_send)
+					{
+						request->req_flags &= ~req_stall;
+						request->req_operation = Request::req_sync;
+						EXE_looper(tdbb, request, request->req_next);
+					}
+					break;
+				}
+			}
+
 			INF_request_info(request, itemsLength, items, bufferLength, buffer);
 		}
 		catch (const Exception& ex)
@@ -4851,6 +4871,7 @@ void JAttachment::transactRequest(CheckStatusWrapper* user_status, ITransaction*
 		JTransaction* const jt = getTransactionInterface(user_status, tra);
 		EngineContextHolder tdbb(user_status, this, FB_FUNCTION);
 
+		Request* request = nullptr;
 		jrd_tra* transaction = jt->getHandle();
 		validateHandle(tdbb, transaction);
 		check_database(tdbb);
@@ -4862,7 +4883,6 @@ void JAttachment::transactRequest(CheckStatusWrapper* user_status, ITransaction*
 			const MessageNode* inMessage = NULL;
 			const MessageNode* outMessage = NULL;
 
-			Request* request = NULL;
 			MemoryPool* new_pool = att->createPool();
 
 			try
@@ -4888,9 +4908,7 @@ void JAttachment::transactRequest(CheckStatusWrapper* user_status, ITransaction*
 			}
 			catch (const Exception&)
 			{
-				if (request)
-					CMP_release(tdbb, request);
-				else
+				if (!request)
 					att->deletePool(new_pool);
 
 				throw;
@@ -4923,6 +4941,15 @@ void JAttachment::transactRequest(CheckStatusWrapper* user_status, ITransaction*
 
 			if (out_msg_length)
 			{
+				// Workaround for GPRE that generated unneeded blr_send
+				if ((request->req_flags & req_active)
+					&& request->req_operation == Request::req_send)
+				{
+					request->req_flags &= ~req_stall;
+					request->req_operation = Request::req_proceed;
+					EXE_looper(tdbb, request, request->req_next);
+				}
+
 				memcpy(out_msg, outMessage->getBuffer(request), out_msg_length);
 			}
 
@@ -4932,6 +4959,9 @@ void JAttachment::transactRequest(CheckStatusWrapper* user_status, ITransaction*
 		}
 		catch (const Exception& ex)
 		{
+			if (request)
+				CMP_release(tdbb, request);
+
 			transliterateException(tdbb, ex, user_status, "JAttachment::transactRequest");
 			return;
 		}

src/jrd/mov.cpp

@@ -438,7 +438,7 @@ Firebird::string MOV_make_string2(Jrd::thread_db* tdbb, const dsc* desc, USHORT
 }
 
 
-void MOV_move(Jrd::thread_db* tdbb, /*const*/ dsc* from, dsc* to)
+void MOV_move(Jrd::thread_db* tdbb, /*const*/ dsc* from, dsc* to, bool trustedSource)
 {
 /**************************************
  *
@@ -454,7 +454,7 @@ void MOV_move(Jrd::thread_db* tdbb, /*const*/ dsc* from, dsc* to)
 	if (DTYPE_IS_BLOB_OR_QUAD(from->dsc_dtype) || DTYPE_IS_BLOB_OR_QUAD(to->dsc_dtype))
 		Jrd::blb::move(tdbb, from, to);
 	else
-		CVT_move(from, to, tdbb->getAttachment()->att_dec_status);
+		CVT_move_common(from, to, tdbb->getAttachment()->att_dec_status, &Jrd::EngineCallbacks::instance, trustedSource);
 }
 
 

src/jrd/mov_proto.h

@@ -52,7 +52,7 @@ USHORT	MOV_make_string(Jrd::thread_db*, const dsc*, USHORT, const char**, vary*,
 ULONG	MOV_make_string2(Jrd::thread_db*, const dsc*, USHORT, UCHAR**, Jrd::MoveBuffer&, bool = true);
 Firebird::string MOV_make_string2(Jrd::thread_db* tdbb, const dsc* desc, USHORT ttype,
 	bool limit = true);
-void	MOV_move(Jrd::thread_db*, /*const*/ dsc*, dsc*);
+void	MOV_move(Jrd::thread_db*, /*const*/ dsc*, dsc*, bool trustedSource = false);
 Firebird::Decimal64 MOV_get_dec64(Jrd::thread_db*, const dsc*);
 Firebird::Decimal128 MOV_get_dec128(Jrd::thread_db*, const dsc*);
 Firebird::Int128 MOV_get_int128(Jrd::thread_db*, const dsc*, SSHORT);