Skip to content
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.

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

[API] Propagation: fix for hex conversion to binary for odd hex strings #2533

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

karusher
Copy link
Contributor

The conversion of hex to binary did not handle odd-length inputs correctly.

The output was zero-padded on the right rather than the left. On little endian machines, the second to last entry received the 0.

For example, "78cfcfec62ae9e9" would result in:

0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x78 0xcf 0xcf 0xec 0x62 0xae 0x9e 0x09

The desired output is:

0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x07 0x8c 0xfc 0xfe 0xc6 0x2a 0xe9 0xe9

I have attached a toy program to reproduce the problem:

test_program.txt

Changes

Please provide a brief description of the changes here.

Corrected conversion of hex to binary for propagation

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@karusher karusher requested a review from a team February 14, 2024 00:10
Copy link

linux-foundation-easycla bot commented Feb 14, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

The conversion of hex to binary did not handle odd-length inputs
correctly.

The output was zero-padded on the right rather than the left. On
little endian machines, the second to last entry received the 0.
Copy link
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. While this method is always used for converting even-length hex strings (trace_id and span_id) to binary, I believe the additional check for odd lengths should not significantly affect performance.

Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix.

@marcalff marcalff changed the title Fix for hex conversion to binary [API] Propagation: fix for hex conversion to binary Feb 14, 2024
@marcalff marcalff changed the title [API] Propagation: fix for hex conversion to binary [API] Propagation: fix for hex conversion to binary for odd hex strings Feb 14, 2024
@marcalff marcalff merged commit c7a88c4 into open-telemetry:main Feb 14, 2024
47 checks passed
@karusher karusher deleted the hex_fix branch February 14, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants