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

Accept non URL-encoded headers #4103

Merged
merged 4 commits into from
Aug 5, 2024

Conversation

xrmx
Copy link
Contributor

@xrmx xrmx commented Aug 2, 2024

Description

Add a liberal boolean parameter to parse_env_headers default to False to keep backward compatibility for the function.
If liberal=True we try to parse the string even if it does comply with the the W3C Baggage HTTP header format to be more compatible with other languages SDKs that accept non URL-encoded headers by default.

liberal is a reference to Postel's law.

Fixes #4102

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A

Does This PR Require a Contrib Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@xrmx xrmx changed the title Accept non urlencoded headers Accept non URL-encoded headers Aug 2, 2024
Copy link

@trentm trentm left a comment

Choose a reason for hiding this comment

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

LGTM, though obviously I'm not a Python approver, nor am I familiar with much of the code in this repo.

opentelemetry-api/src/opentelemetry/util/re.py Outdated Show resolved Hide resolved
@xrmx xrmx force-pushed the accept-non-urlencoded-headers branch from cee9dc3 to 6a6830b Compare August 5, 2024 08:20
Add a liberal boolean parameter to parse_env_headers default to False to
keep backward compatibility for the function.
If liberal=True we try to parse the string even if it does comply with
the the W3C Baggage HTTP header format to be more compatible with other
other languages SDKs that accept non URL-encoded headers by default.
@xrmx xrmx force-pushed the accept-non-urlencoded-headers branch from 96e4db8 to 4ceb51d Compare August 5, 2024 09:26
@xrmx xrmx marked this pull request as ready for review August 5, 2024 09:28
@xrmx xrmx requested a review from a team August 5, 2024 09:28
@lzchen lzchen merged commit 57d30b3 into open-telemetry:main Aug 5, 2024
284 checks passed
xrmx added a commit to xrmx/opentelemetry-python that referenced this pull request Aug 19, 2024
After open-telemetry#4103 we accept non url-encoded headers for OTLP. Naively this
implementation only accepts digits, ascii letters, underscores (\w) and spaces.
Instead broaden the accepted chars to what we accept for urlencoded
values so that we accepts = to allow for base64 encoded values.
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.

Supporting non URL-encoded OTEL_EXPORTER_OTLP_HEADERS
3 participants