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

socket: socket::set_sockaddr() for IPv4 addresses in IPv6 builds #7196

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

dwmw2
Copy link
Contributor

@dwmw2 dwmw2 commented Aug 5, 2024

While testing TheStaticTurtle/esphome_syslog#17 I spotted that logging to Legacy IP addresses while IPv6 is enabled doesn't work, because socket::set_sockaddr() still populates a sockaddr_sin6.

When passed a Legacy IP address as the input address string, set_sockaddr() should populate the output with a struct sockaddr_in.

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)
  • Other

Related issue or feature (if applicable): fixes

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

Test Environment

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

Example entry for config.yaml:

# Example config.yaml

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:

@probot-esphome
Copy link

probot-esphome bot commented Aug 5, 2024

Hey there @esphome/core, mind taking a look at this pull request as it has been labeled with an integration (socket) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.90%. Comparing base (4d8b5ed) to head (d166213).
Report is 1072 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #7196      +/-   ##
==========================================
+ Coverage   53.70%   53.90%   +0.19%     
==========================================
  Files          50       50              
  Lines        9408     9653     +245     
  Branches     1654     1706      +52     
==========================================
+ Hits         5053     5203     +150     
- Misses       4056     4126      +70     
- Partials      299      324      +25     

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

dwmw2 added a commit to dwmw2/esphome_syslog that referenced this pull request Aug 5, 2024
Using a Legacy IP syslog server in an IPv6-enabled build fails due to a
socket::set_sockaddr() bug. Just do it ourselves for now, until it's fixed
by esphome/esphome#7196
@dwmw2 dwmw2 force-pushed the set_sockaddr_fix branch from d816f87 to e76712c Compare August 6, 2024 00:10
When passed a Legacy IP address as the input address string,
set_sockaddr() should populate the output with a struct sockaddr_in.
@dwmw2 dwmw2 force-pushed the set_sockaddr_fix branch from e76712c to d166213 Compare August 6, 2024 00:12
@jesserockz jesserockz changed the title socket: socket::set_sockaddr() for Legacy IP addresses in IPv6 builds socket: socket::set_sockaddr() for IPv4 addresses in IPv6 builds Aug 6, 2024
@jesserockz jesserockz merged commit 3ba9caa into esphome:dev Aug 6, 2024
31 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 8, 2024
@dwmw2 dwmw2 deleted the set_sockaddr_fix branch August 16, 2024 14:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants