Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Fix Unicode string null termination in StringToBytes#6

Merged
rkttu merged 3 commits into
master from
copilot/fix-queue-label-issue
Nov 30, 2025
Merged

Fix Unicode string null termination in StringToBytes #6
rkttu merged 3 commits into
master from
copilot/fix-queue-label-issue

Conversation

Copilot AI commented Nov 30, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Setting MessageQueue.Label was not persisting to MSMQ because StringToBytes produced malformed Unicode strings.

Problem

Unicode (UTF-16LE) strings require a 2-byte null terminator. The original implementation allocated length * 2 + 1 bytes and only set 1 null byte, causing MSMQ's MQSetQueueProperties to receive improperly terminated strings.

Fix

  • Buffer size: value.Length * 2 + 1 → value.Length * 2 + 2
  • Null terminator: Set both final bytes to 0
// Before
int size = value.Length * 2 + 1;
byte[] byteBuffer = new byte[size];
byteBuffer[size - 1] = 0;
// After 
int size = value.Length * 2 + 2;
byte[] byteBuffer = new byte[size];
byteBuffer[size - 2] = 0;
byteBuffer[size - 1] = 0;

Affected APIs

All properties using StringToBytes: MessageQueue.Label, MessageQueue.MulticastAddress, Message.Label, Message.AuthenticationProviderName, queue format names.

Documentation

Added a disclaimer to README.md clarifying that:

  • This source code is based on Microsoft's Reference Source but may contain differences
  • Bug fixes are applied in the public interest based on community-reported issues
  • If Microsoft officially releases an MSMQ client package, that should take priority

Added release note for v1.2.0 documenting this fix.

Original prompt

This section details on the original issue you should resolve

<issue_title>Label is not working as expected</issue_title>
<issue_description>Hi

Thanks for nice code, this will be very useful for me.
When I update "Label", this is not reflected in the que.

Here is my sample code

MessageQueue messageQueue = null;
if (MessageQueue.Exists(@".\Private$\SomeTestName"))
{
messageQueue = new MessageQueue(@".\Private$\SomeTestName");
messageQueue.Label = "Testing Queue";
}
else
{
// Create the Queue
MessageQueue.Create(@".\Private$\SomeTestName");
messageQueue = new MessageQueue(@".\Private$\SomeTestName");
messageQueue.Label = "Newly Created Queue";
}
messageQueue.Send("First ever Message is sent to MSMQ", "Title");

Is there something I do wrong or is this an bug in the code?

LP</issue_description>

Comments on the Issue (you are @copilot in this section)

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

 

Co-authored-by: rkttu <1297346+rkttu@users.noreply.github.com>
Copilot AI changed the title (削除) [WIP] Fix issue with message queue label not updating (削除ここまで) (追記) Fix Unicode string null termination in StringToBytes (追記ここまで) Nov 30, 2025
Copilot AI requested a review from rkttu November 30, 2025 12:05

rkttu commented Nov 30, 2025

Copy link
Copy Markdown
Contributor

@copilot 좋아. 그런데 이 코드는 Microsoft Reference Source로부터 온 코드이고, 내가 이 코드를 직접 수정할 권한은 없다고 생각해. 그럼에도 이 fix를 넣는게 맞는지 검토가 필요해.

Copilot AI commented Nov 30, 2025

Copy link
Copy Markdown
Contributor Author

@copilot 좋아. 그런데 이 코드는 Microsoft Reference Source로부터 온 코드이고, 내가 이 코드를 직접 수정할 권한은 없다고 생각해. 그럼에도 이 fix를 넣는게 ��...

이 코드가 Microsoft Reference Source에서 온 것은 맞습니다. 하지만 MIT 라이선스로 배포되어 있어 "modify" 권한이 명시적으로 허용됩니다.

또한 이 수정은 원본 코드의 버그입니다. Unicode (UTF-16LE) 문자열은 2바이트 null 종단자가 필요한데, 원본 코드는 1바이트만 할당했습니다. 이는 MSMQ API가 문자열을 제대로 인식하지 못하게 만드는 명백한 오류입니다.

결정은 프로젝트 메인테이너께서 해주시면 됩니다. 만약 이 수정을 포함하지 않기로 결정하시면 PR을 닫아주세요.

rkttu commented Nov 30, 2025

Copy link
Copy Markdown
Contributor

@copilot 좋아. 이미 이 패키지가 쓰이는 곳이 있으니, "공익"차원에서 PR을 반영하고 릴리스를 준비하는게 타당할까?

Copilot AI commented Nov 30, 2025

Copy link
Copy Markdown
Contributor Author

@copilot 좋아. 이미 이 패키지가 쓰이는 곳이 있으니, "공익"차원에서 PR을 반영하고 릴리스를 준비하는게 타당할까?

네, 타당합니다. 이 버그는 Label, MulticastAddress 등 여러 속성에 영향을 미치는 문제이고, 현재 패키지 사용자들이 동일한 문제를 겪고 있을 가능성이 높습니다. 수정 내용도 최소화되어 있어 기존 동작에 부정적 영향을 줄 위험이 낮습니다.

rkttu commented Nov 30, 2025

Copy link
Copy Markdown
Contributor

@copilot 좋아. 그러면 README.md 파일에 수정 이력과 함께, 이 소스 코드는 Microsoft가 Reference Source를 통해 공개했던 것에서 차이가 나는 점이 있으나, 공익 관점에서 버그를 제보받아 수정하기로 결정한 것으로, 여전히 Microsoft가 정식으로 MSMQ 클라이언트 패키지를 게시하기로 결정한다면 그쪽이 우선 순위가 더 높다는 사실을 적극적으로 소명하는 disclaimer를 추가했으면 해.

 

Co-authored-by: rkttu <1297346+rkttu@users.noreply.github.com>

Copilot AI commented Nov 30, 2025

Copy link
Copy Markdown
Contributor Author

@copilot 좋아. 그러면 README.md 파일에 수정 이력과 함께, 이 소스 코드는 Microsoft가 Reference Source를 통해 공개했던 것에서 차이가 나는 점이 있으나, 공익 ��...

README.md에 Disclaimer 섹션과 v1.2.0 릴리스 노트를 추가했습니다 (748dbd3).

Disclaimer에는 다음 내용이 포함되어 있습니다:

  • 이 코드가 Microsoft Reference Source 기반이지만 차이가 있을 수 있음
  • 커뮤니티 보고 기반으로 공익 차원에서 버그 수정이 적용됨
  • Microsoft가 공식 MSMQ 클라이언트 패키지를 출시하면 그쪽이 우선순위가 높음

@rkttu rkttu marked this pull request as ready for review November 30, 2025 12:18
Copilot AI review requested due to automatic review settings November 30, 2025 12:18
@rkttu rkttu merged commit 0a75545 into master Nov 30, 2025
2 checks passed
@rkttu rkttu deleted the copilot/fix-queue-label-issue branch November 30, 2025 12:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a critical bug in Unicode string handling that prevented MessageQueue.Label and other string properties from persisting correctly to MSMQ. The root cause was improper null termination in UTF-16LE encoded strings.

Key Changes:

  • Fixed StringToBytes method to allocate proper buffer size (value.Length * 2 + 2 instead of value.Length * 2 + 1) for 2-byte Unicode null terminator
  • Added disclaimer to README clarifying this is a community-maintained port with bug fixes
  • Documented the fix in release notes for v1.2.0

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
Experimental.System.Messaging/Messaging/Message.cs Fixed buffer allocation and null termination in StringToBytes to properly handle UTF-16LE encoding requirements
README.md Added disclaimer about community maintenance and documented the Unicode fix in v1.2.0 release notes
Comments suppressed due to low confidence (19)

Experimental.System.Messaging/Messaging/Message.cs:2218

  • 'this' used in lock statement.
 lock (this)

Experimental.System.Messaging/Messaging/Message.cs:1658

  • Replace this call with a call to managed code if possible.
 SafeNativeMethods.MQFreeMemory(memoryHandle);

Experimental.System.Messaging/Messaging/Message.cs:471

  • Condition always evaluates to 'false'.
 if (value)

Experimental.System.Messaging/Messaging/Message.cs:2075

  • Condition always evaluates to 'true'.
 if (!value)

Experimental.System.Messaging/Messaging/Message.cs:2351

  • Redundant call to 'ToString' on a String object.
 result.Append((new Guid(guidBytes)).ToString());

Experimental.System.Messaging/Messaging/Message.cs:343

  • These 'if' statements can be combined.
 if (this.cachedAdminQueue == null)
 {
 if (properties.GetUI4(NativeMethods.MESSAGE_PROPID_ADMIN_QUEUE_LEN) != 0)
 {
 string queueFormatName = StringFromBytes(properties.GetString(NativeMethods.MESSAGE_PROPID_ADMIN_QUEUE),
 properties.GetUI4(NativeMethods.MESSAGE_PROPID_ADMIN_QUEUE_LEN));
 this.cachedAdminQueue = new MessageQueue("FORMATNAME:" + queueFormatName);
 }
 }

Experimental.System.Messaging/Messaging/Message.cs:875

  • These 'if' statements can be combined.
 if (this.cachedDestinationQueue == null)
 {
 if (this.properties.GetUI4(NativeMethods.MESSAGE_PROPID_DEST_QUEUE_LEN) != 0)
 {
 string queueFormatName = StringFromBytes(properties.GetString(NativeMethods.MESSAGE_PROPID_DEST_QUEUE),
 properties.GetUI4(NativeMethods.MESSAGE_PROPID_DEST_QUEUE_LEN));
 this.cachedDestinationQueue = new MessageQueue("FORMATNAME:" + queueFormatName);
 }
 }

Experimental.System.Messaging/Messaging/Message.cs:1444

  • These 'if' statements can be combined.
 if (this.cachedResponseQueue == null)
 {
 if (properties.GetUI4(NativeMethods.MESSAGE_PROPID_RESP_QUEUE_LEN) != 0)
 {
 string queueFormatName = StringFromBytes(properties.GetString(NativeMethods.MESSAGE_PROPID_RESP_QUEUE),
 properties.GetUI4(NativeMethods.MESSAGE_PROPID_RESP_QUEUE_LEN));
 this.cachedResponseQueue = new MessageQueue("FORMATNAME:" + queueFormatName);
 }
 }

Experimental.System.Messaging/Messaging/Message.cs:1824

  • These 'if' statements can be combined.
 if (this.cachedTransactionStatusQueue == null)
 {
 if (this.properties.GetUI4(NativeMethods.MESSAGE_PROPID_XACT_STATUS_QUEUE_LEN) != 0)
 {
 string queueFormatName = StringFromBytes(properties.GetString(NativeMethods.MESSAGE_PROPID_XACT_STATUS_QUEUE),
 properties.GetUI4(NativeMethods.MESSAGE_PROPID_XACT_STATUS_QUEUE_LEN));
 this.cachedTransactionStatusQueue = new MessageQueue("FORMATNAME:" + queueFormatName);
 }
 }

Experimental.System.Messaging/Messaging/Message.cs:2297

  • These 'if' statements can be combined.
 if (this.filter.AdministrationQueue)
 {
 if (properties.GetUI4(NativeMethods.MESSAGE_PROPID_ADMIN_QUEUE_LEN) == 0)
 {
 this.properties.Ghost(NativeMethods.MESSAGE_PROPID_ADMIN_QUEUE);
 this.properties.Ghost(NativeMethods.MESSAGE_PROPID_ADMIN_QUEUE_LEN);
 }
 }

Experimental.System.Messaging/Messaging/Message.cs:2303

  • These 'if' statements can be combined.
 if (this.filter.EncryptionAlgorithm)
 {
 if ((this.filter.UseEncryption && !this.UseEncryption) || !this.filter.UseEncryption)
 this.properties.Ghost(NativeMethods.MESSAGE_PROPID_ENCRYPTION_ALG);
 }

Experimental.System.Messaging/Messaging/Message.cs:2311

  • These 'if' statements can be combined.
 if (this.filter.DigitalSignature)
 {
 if (properties.GetUI4(NativeMethods.MESSAGE_PROPID_SIGNATURE_LEN) == 0)
 {
 this.properties.Ghost(NativeMethods.MESSAGE_PROPID_SIGNATURE);
 this.properties.Ghost(NativeMethods.MESSAGE_PROPID_SIGNATURE_LEN);
 }
 }

Experimental.System.Messaging/Messaging/Message.cs:2319

  • These 'if' statements can be combined.
 if (this.filter.DestinationSymmetricKey)
 {
 if (properties.GetUI4(NativeMethods.MESSAGE_PROPID_DEST_SYMM_KEY_LEN) == 0)
 {
 this.properties.Ghost(NativeMethods.MESSAGE_PROPID_DEST_SYMM_KEY);
 this.properties.Ghost(NativeMethods.MESSAGE_PROPID_DEST_SYMM_KEY_LEN);
 }
 }

Experimental.System.Messaging/Messaging/Message.cs:2327

  • These 'if' statements can be combined.
 if (this.filter.ResponseQueue)
 {
 if (properties.GetUI4(NativeMethods.MESSAGE_PROPID_RESP_QUEUE_LEN) == 0)
 {
 this.properties.Ghost(NativeMethods.MESSAGE_PROPID_RESP_QUEUE);
 this.properties.Ghost(NativeMethods.MESSAGE_PROPID_RESP_QUEUE_LEN);
 }
 }

Experimental.System.Messaging/Messaging/Message.cs:2335

  • These 'if' statements can be combined.
 if (this.filter.TransactionStatusQueue)
 {
 if (properties.GetUI4(NativeMethods.MESSAGE_PROPID_XACT_STATUS_QUEUE_LEN) == 0)
 {
 this.properties.Ghost(NativeMethods.MESSAGE_PROPID_XACT_STATUS_QUEUE);
 this.properties.Ghost(NativeMethods.MESSAGE_PROPID_XACT_STATUS_QUEUE_LEN);
 }
 }

Experimental.System.Messaging/Messaging/Message.cs:55

  • Field 'properties' can be 'readonly'.
 internal MessagePropertyVariants properties;

Experimental.System.Messaging/Messaging/Message.cs:45

  • Field 'filter' can be 'readonly'.
 private MessagePropertyFilter filter;

Experimental.System.Messaging/Messaging/Message.cs:529

  • Both branches of this 'if' statement return - consider using '?' to express intent better.
 if (this.properties.GetUI4(NativeMethods.MESSAGE_PROPID_PROV_NAME_LEN) != 0)
 return StringFromBytes(this.properties.GetString(NativeMethods.MESSAGE_PROPID_PROV_NAME),
 properties.GetUI4(NativeMethods.MESSAGE_PROPID_PROV_NAME_LEN));
 else
 return "";

Experimental.System.Messaging/Messaging/Message.cs:1241

  • Both branches of this 'if' statement return - consider using '?' to express intent better.
 if (properties.GetUI4(NativeMethods.MESSAGE_PROPID_LABEL_LEN) != 0)
 return StringFromBytes(this.properties.GetString(NativeMethods.MESSAGE_PROPID_LABEL),
 properties.GetUI4(NativeMethods.MESSAGE_PROPID_LABEL_LEN));
 else
 return "";

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +2422 to +2423
byteBuffer[size - 2] = 0;
byteBuffer[size - 1] = 0;

Copilot AI Nov 30, 2025

Copy link

Choose a reason for hiding this comment

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

The explicit setting of the last two bytes to 0 is redundant since byte arrays in C# are zero-initialized by default. While this doesn't affect correctness, it can be simplified:

internal static byte[] StringToBytes(string value)
{
 int size = value.Length * 2 + 2;
 byte[] byteBuffer = new byte[size];
 Encoding.Unicode.GetBytes(value.ToCharArray(), 0, value.Length, byteBuffer, 0);
 // Last 2 bytes are already 0 by default
 return byteBuffer;
}

Alternatively, if you want to make the null terminator explicit for clarity, consider adding a comment explaining why.

Suggested change
byteBuffer[size - 2] = 0;
byteBuffer[size - 1] = 0;
// Last 2 bytes are already 0 by default (null terminator for Unicode string)

Copilot uses AI. Check for mistakes.
Comment on lines 2417 to 2424
internal static byte[] StringToBytes(string value)
{
int size = value.Length * 2 + 1;
int size = value.Length * 2 + 2;
byte[] byteBuffer = new byte[size];
byteBuffer[size - 1] = 0;
Encoding.Unicode.GetBytes(value.ToCharArray(), 0, value.Length, byteBuffer, 0);
byteBuffer[size - 2] = 0;
byteBuffer[size - 1] = 0;
return byteBuffer;

Copilot AI Nov 30, 2025

Copy link

Choose a reason for hiding this comment

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

The StringToBytes method lacks test coverage for this critical bug fix. Consider adding unit tests to verify:

  1. Correct 2-byte null termination for Unicode strings
  2. Empty string handling
  3. Strings with special characters
  4. Round-trip consistency with StringFromBytes

This is especially important since this bug affected multiple user-facing APIs (MessageQueue.Label, Message.Label, etc.) and caused data persistence issues with MSMQ.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@rkttu rkttu Awaiting requested review from rkttu

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Label is not working as expected

3 participants

AltStyle によって変換されたページ (->オリジナル) /