Skip to content

fix: use correct header format in Test_Blocking_client_ip_with_forwarded #5134

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Aug 27, 2025
Merged

fix: use correct header format in Test_Blocking_client_ip_with_forwarded #5134

merged 11 commits into from
Aug 27, 2025

Conversation

@RomainMuller
Copy link
Contributor

@RomainMuller RomainMuller commented Aug 26, 2025 •
edited
Loading

Motivation

Following implementation in DataDog/dd-trace-go#3911

Changes

Uses the correct format for a Forwarded header value.
Add IPv6 format test.

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed and the CI green, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • If PR title starts with [<language>], double-check that only <language> is impacted by the change
  • No system-tests internal is modified. Otherwise, I have the approval from R&P team
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added (or removed)?

@RomainMuller RomainMuller changed the title [golang] enable Test_Blocking_client_ip_with_forwarded fix: use correct header format in Test_Blocking_client_ip_with_forwarded Aug 26, 2025
@RomainMuller RomainMuller force-pushed the romain.marcadier/APPSEC-58265/clientip.forwarded branch from f089515 to 5aca1f5 Compare August 26, 2025 09:23
@christophe-papazian christophe-papazian mentioned this pull request Aug 26, 2025
Merged
2 tasks
@RomainMuller RomainMuller marked this pull request as ready for review August 26, 2025 14:01
@RomainMuller RomainMuller requested review from a team as code owners August 26, 2025 14:01
@RomainMuller RomainMuller requested review from christophe-papazian and wconti27 and removed request for a team August 26, 2025 14:01
christophe-papazian added a commit to DataDog/dd-trace-py that referenced this pull request Aug 27, 2025
@christophe-papazian
chore(tracer): add forwarded ip header format support (#14418)
3ec641e 
Following #14400

This PR improves the support for `forwarded` header by adding ip format
extraction from the forwarded format described in
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Forwarded
https://www.rfc-editor.org/rfc/rfc7239

- We only extract the ip from the `for` parameter.
- Also add tests for different possible ip formats (ipv4, ipv6, quoted
or unquoted, with or without port)

This will also be supported by system tests
DataDog/system-tests#5134

APPSEC-58261

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
@RomainMuller RomainMuller enabled auto-merge (squash) August 27, 2025 07:55
@cbeauchesne cbeauchesne disabled auto-merge August 27, 2025 11:41
@cbeauchesne cbeauchesne merged commit 2df2351 into main Aug 27, 2025
852 of 854 checks passed
@cbeauchesne cbeauchesne deleted the romain.marcadier/APPSEC-58265/clientip.forwarded branch August 27, 2025 11:41
taegyunkim pushed a commit to DataDog/dd-trace-py that referenced this pull request Sep 3, 2025
@christophe-papazian @taegyunkim
chore(tracer): add forwarded ip header format support (#14418)
367a2f5 
Following #14400

This PR improves the support for `forwarded` header by adding ip format
extraction from the forwarded format described in
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Forwarded
https://www.rfc-editor.org/rfc/rfc7239

- We only extract the ip from the `for` parameter.
- Also add tests for different possible ip formats (ipv4, ipv6, quoted
or unquoted, with or without port)

This will also be supported by system tests
DataDog/system-tests#5134

APPSEC-58261

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
erikayasuda pushed a commit to DataDog/dd-trace-py that referenced this pull request Sep 3, 2025
@christophe-papazian @erikayasuda
chore(tracer): add forwarded ip header format support (#14418)
f603a3b 
Following #14400

This PR improves the support for `forwarded` header by adding ip format
extraction from the forwarded format described in
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Forwarded
https://www.rfc-editor.org/rfc/rfc7239

- We only extract the ip from the `for` parameter.
- Also add tests for different possible ip formats (ipv4, ipv6, quoted
or unquoted, with or without port)

This will also be supported by system tests
DataDog/system-tests#5134

APPSEC-58261

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@darccio darccio darccio approved these changes

@christophe-papazian christophe-papazian christophe-papazian approved these changes

@wconti27 wconti27 Awaiting requested review from wconti27 wconti27 was automatically assigned from DataDog/apm-python

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@RomainMuller @darccio @christophe-papazian @cbeauchesne