Skip to content

Conversation

@bdraco
Copy link
Member

@bdraco bdraco commented Dec 14, 2025

What does this implement/fix?

Fix getpeername() in LWIP raw TCP socket implementation to return the remote peer's address instead of the local address.

The sockaddr overload of getpeername() was incorrectly using pcb_->local_ip and pcb_->local_port instead of pcb_->remote_ip and pcb_->remote_port. This made it return the same result as getsockname(), which is incorrect:

  • getsockname() should return the local address (our side)
  • getpeername() should return the remote address (peer's side)

This was a copy-paste bug - the string overloads were already correct:

std::string getpeername() { return this->format_ip_address_(pcb_->remote_ip); }  // Correct
std::string getsockname() { return this->format_ip_address_(pcb_->local_ip); }   // Correct

But the sockaddr overloads were identical:

// Before (wrong):
int getpeername(...) { return this->ip2sockaddr_(&pcb_->local_ip, pcb_->local_port, ...); }
int getsockname(...) { return this->ip2sockaddr_(&pcb_->local_ip, pcb_->local_port, ...); }

// After (fixed):
int getpeername(...) { return this->ip2sockaddr_(&pcb_->remote_ip, pcb_->remote_port, ...); }
int getsockname(...) { return this->ip2sockaddr_(&pcb_->local_ip, pcb_->local_port, ...); }

Types of changes

  • Bugfix (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)
  • Developer breaking change (an API change that could break external components)
  • Code quality improvements to existing code or addition of tests
  • Other

Related issue or feature (if applicable):

  • None

Pull request in esphome-docs with documentation (if applicable):

  • N/A

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx
  • nRF52840

Example entry for config.yaml:

# No configuration changes - this is an internal socket API fix

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@github-actions
Copy link
Contributor

To use the changes from this PR as an external component, add the following to your ESPHome configuration YAML file:

external_components:
  - source: github://pr#12475
    components: [socket]
    refresh: 1h

(Added by the PR bot)

@codecov-commenter
Copy link

codecov-commenter commented Dec 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.57%. Comparing base (e0ce66e) to head (512a7df).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #12475      +/-   ##
==========================================
- Coverage   72.60%   72.57%   -0.03%     
==========================================
  Files          53       53              
  Lines       11192    11192              
  Branches     1517     1517              
==========================================
- Hits         8126     8123       -3     
- Misses       2674     2676       +2     
- Partials      392      393       +1     

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

@bdraco bdraco marked this pull request as ready for review December 15, 2025 03:46
@bdraco bdraco requested a review from a team as a code owner December 15, 2025 03:46
Copilot AI review requested due to automatic review settings December 15, 2025 03:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a copy-paste bug in the LWIP raw TCP socket implementation where getpeername() was incorrectly returning the local address instead of the remote peer's address. The sockaddr overload was mistakenly using pcb_->local_ip and pcb_->local_port instead of pcb_->remote_ip and pcb_->remote_port, making it behave identically to getsockname(). This violated standard socket API semantics and was inconsistent with the string overload of getpeername() which was already correct.

Key Changes

  • Fixed getpeername(struct sockaddr *name, socklen_t *addrlen) to use pcb_->remote_ip and pcb_->remote_port instead of pcb_->local_ip and pcb_->local_port

@bdraco
Copy link
Member Author

bdraco commented Dec 16, 2025

Thanks

@bdraco bdraco merged commit ead60bc into dev Dec 16, 2025
50 checks passed
@bdraco bdraco deleted the lwip_raw_tcp_impl_getpeername branch December 16, 2025 06:48
@bdraco bdraco added this to the 2025.12.0b5 milestone Dec 16, 2025
swoboda1337 pushed a commit that referenced this pull request Dec 16, 2025
@swoboda1337 swoboda1337 mentioned this pull request Dec 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants