Skip to content

Conversation

batuhansk
Copy link
Contributor

Description

This PR improves the W3C Baggage implementation to better align with the specification requirements, focusing on proper UTF-8 encoding and RFC7230 validation.

Changes

  • Implement proper UTF-8 percent encoding for baggage values
  • Add RFC7230 token validation for baggage keys (using allowed characters: !#$%&'*+-.^_|~` and alphanumeric)
  • Remove ASCII-only restriction from baggage values to allow any UTF-8 characters
  • Maintain ASCII validation for resource attributes (separate concern from baggage)

Key Implementation Details

  • EntryKey: Now validates against RFC7230 token specification
  • EntryValue: Allows any UTF-8 characters in values (will be percent-encoded during propagation)
  • W3CBaggagePropagator: Implements proper percent encoding/decoding
  • Tests: Added comprehensive W3C specification compliance tests

Notes

  • Removed previously proposed 180-byte length limit after reviewing other language implementations and considering potential data loss implications
  • Maintains backward compatibility while improving spec compliance
  • Added extensive test coverage for edge cases and W3C spec examples

References

batuhansk added 2 commits May 6, 2025 15:49
This commit improves the W3C Baggage implementation to better align with the
specification requirements:

- Implement proper UTF-8 percent encoding for baggage values
- Add RFC7230 token validation for baggage keys
- Remove ASCII-only restriction from baggage values
- Add 180-byte length limit for baggage strings
- Maintain ASCII validation for resource attributes
- Add comprehensive test coverage for W3C spec compliance

Key changes:
* EntryKey: Add RFC7230 token character validation
* EntryValue: Allow any UTF-8 characters in values
* W3CBaggagePropagator: Implement proper percent encoding/decoding
* Tests: Add extensive W3C specification compliance tests

fixes open-telemetry#761
- Consolidate and restructure EntryKey tests for better organization
- Add comprehensive RFC7230 token validation test cases
- Add whitespace trimming and case sensitivity tests
- Remove redundant unprintable character tests
- Improve test documentation and readability
@batuhansk batuhansk requested a review from bryce-b May 6, 2025 18:48
Copy link

codecov bot commented May 6, 2025

Codecov Report

Attention: Patch coverage is 98.18182% with 1 line in your changes missing coverage. Please review.

Project coverage is 68.18%. Comparing base (7bad8ae) to head (e72669a).
Report is 90 commits behind head on main.

Files with missing lines Patch % Lines
...Api/Baggage/Propagation/W3CBaggagePropagator.swift 97.56% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #763      +/-   ##
==========================================
+ Coverage   67.89%   68.18%   +0.29%     
==========================================
  Files         344      326      -18     
  Lines       15169    14599     -570     
==========================================
- Hits        10299     9955     -344     
+ Misses       4870     4644     -226     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nachoBonafonte nachoBonafonte merged commit 0576361 into open-telemetry:main May 6, 2025
11 checks passed
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.

W3CBaggagePropagator Missing Percent Encoding for Special Characters in Values
3 participants