Skip to content

Conversation

@leandrodamascena
Copy link
Contributor

Issue number: closes #7288

Summary

Changes

This PR add supports for IP address with port (e.g., 10.1.15.242:39870) in APIGateway models.

User experience

from aws_lambda_powertools.utilities.parser import event_parser
from aws_lambda_powertools.utilities.parser.models import APIGatewayProxyEventV2Model
from aws_lambda_powertools.utilities.typing import LambdaContext


@event_parser
def lambda_handler(event: APIGatewayProxyEventV2Model, context: LambdaContext):
    # Access sourceIp - now works with both formats:
    # - Standard IP: "192.168.1.1" 
    # - IP with port: "10.1.15.242:39870"
    source_ip = event.requestContext.http.sourceIp
    
    print(f"Request from: {source_ip}")
    
    return {
        'statusCode': 200,
        'body': f'Hello from {source_ip}!'
    }

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@leandrodamascena leandrodamascena requested a review from a team as a code owner September 4, 2025 13:09
@boring-cyborg boring-cyborg bot added the tests label Sep 4, 2025
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 4, 2025
@leandrodamascena leandrodamascena self-assigned this Sep 4, 2025
@github-actions github-actions bot added the feature New feature or functionality label Sep 4, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 4, 2025

Please retry analysis of this Pull-Request directly on SonarQube Cloud

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 4, 2025

Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

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

Nice, I'll open an issue for TypeScript to do the same.

@codecov
Copy link

codecov bot commented Sep 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.36%. Comparing base (02b2305) to head (2b15fc2).
⚠️ Report is 3 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #7315   +/-   ##
========================================
  Coverage    96.35%   96.36%           
========================================
  Files          275      275           
  Lines        12980    13002   +22     
  Branches       965      966    +1     
========================================
+ Hits         12507    12529   +22     
  Misses         366      366           
  Partials       107      107           

☔ 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.

@leandrodamascena leandrodamascena merged commit 6e40b3d into develop Sep 4, 2025
14 checks passed
@leandrodamascena leandrodamascena deleted the fix/apigw-sourceip-bug branch September 4, 2025 13:26
IPvAnyNetwork(value)
except ValueError:
try:
ip_part = value.split(":")[0]
Copy link

Choose a reason for hiding this comment

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

Has "IPv6-with-port" been taken into account?

Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I understand, the IPvAnyAddress model we're using supports both v4 and v6 - source

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I think this can break ipv6 yes in some situations. I need to run some tests before confirming this.

Thanks @iBug

Copy link
Contributor

Choose a reason for hiding this comment

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

But I think the way we split the port might actually not work with IPv6 which contains multiple :. To make it work we should've taken everything but the last item of the split list instead of just the first. cc @leandrodamascena

Copy link

Choose a reason for hiding this comment

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

But I think this can break ipv6 yes in some situations.

That's what I meant. I don't think it would parse when value.split(":")[0] returns something like [2001. A better approach might be

ip_part = value.rsplit(":", 1)[0]

Copy link

Choose a reason for hiding this comment

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

And probably with .strip("[]") as well, if IPvAnyNetwork doesn't expected bracketed IPv6 addresses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reoepned this issue: #7288

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or functionality size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: support IP address with port in requestContext.http.sourceIp when parsing requests via TLS

3 participants