GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-74895: getaddrinfo no longer raises OverflowError by smejkar · Pull Request #2435 · python/cpython

Conversation

@smejkar

@smejkar smejkar commented Jun 27, 2017 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

http://bugs.python.org/issue30710

Depends on http://bugs.python.org/issue30711 to pass test_getaddrinfo_overflow.

  • Issue: getaddrinfo raises OverflowError #74895

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@smejkar
bpo-30710: getaddrinfo does not raise OverflowError
fe317a4 
Convert a numeric port number to a string without an intermediate C long.
This prevents raising OverflowError if the port number is out of the
C long range.
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jun 27, 2017
@smejkar

smejkar commented Jun 27, 2017

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka to pass checks, should I (a) wait till #2436 is merged, (b) relax test_getaddrinfo_overflow, or (c) rebase this onto #2436?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Mariatta Mariatta added needs rebase and removed needs rebase labels Oct 9, 2017
@brettcannon brettcannon added the awaiting review label Feb 2, 2018
MaxwellDupre
MaxwellDupre suggested changes Feb 18, 2022
View reviewed changes

@MaxwellDupre MaxwellDupre left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Test output:
`FAIL: test_getaddrinfo_overflow (test.test_socket.GeneralModuleTests)

Traceback (most recent call last):
File "/home/me/Documents/cpython/Lib/test/test_socket.py", line 1370, in test_getaddrinfo_overflow
socket.getaddrinfo(None, _testcapi.LONG_MAX + 1)
AssertionError: gaierror not raised
`
Please fix.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Feb 18, 2022
@github-actions

github-actions Bot commented Mar 21, 2022

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Mar 21, 2022
@ezio-melotti ezio-melotti removed the CLA signed label Jul 13, 2022
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Jul 28, 2022
@github-actions

github-actions Bot commented Aug 29, 2022

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Aug 29, 2022
@arhadthedev arhadthedev changed the title bpo-30710: getaddrinfo raises OverflowError gh-74895: getaddrinfo raises OverflowError Feb 13, 2023
@arhadthedev arhadthedev added extension-modules C modules in the Modules dir topic-IO labels Feb 13, 2023
arhadthedev added 2 commits February 13, 2023 22:09
@arhadthedev
Merge branch 'main' into bpo-30710
06de3c3
@arhadthedev
Add a news entry
874cb15
@arhadthedev

arhadthedev commented Feb 13, 2023

Copy link
Copy Markdown
Member

@giampaolo, @gpshead (as network experts)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@arhadthedev
Clean up after an incorrect merge
2175a89
@smejkar smejkar mannequin mentioned this pull request Feb 13, 2023
getaddrinfo raises OverflowError #74895
Closed
@gpshead gpshead self-assigned this Feb 13, 2023
gpshead added 2 commits February 13, 2023 15:28
@gpshead
Merge branch 'main' into bpo-30710
399c274
@gpshead
use out of range service/port values across all platforms
2ec8318 
Some systems treat it as unsigned internally so ULONG_MAX+1 is needed, not LONG_MAX+1.
@gpshead gpshead added the type-bug An unexpected behavior, bug, or error label Feb 13, 2023
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Feb 14, 2023
@gpshead
Rework the test to allow gaierror or no-error.
7328057 
Also updates our canned getaddrinfo.c implementation to actually
report an error for values outside of the u_short range it casts
the value to instead of ignoring them.
@gpshead gpshead changed the title gh-74895: getaddrinfo raises OverflowError gh-74895: getaddrinfo no longer raises OverflowError Feb 14, 2023
gpshead added 2 commits February 13, 2023 17:07
@gpshead
editing typo fix
ba80744
@gpshead
lol typo
5bfdda4
@gpshead

gpshead commented Feb 14, 2023

Copy link
Copy Markdown
Member

@serhiy-storchaka to pass checks, should I (a) wait till #2436 is merged, (b) relax test_getaddrinfo_overflow, or (c) rebase this onto #2436?

I effectively reworked the test, we don't actually want an error behavior on specific numeric values as we had in the past, we just want to report and error if the underlying platform's getaddrinfo() call reports an error. Not all of them do, or at least not all at the same values.

for all practical purposes I doubt it matters much. it'd be nice for values outside of 16-bit unsigned to be rejected, but that only makes strict sense for IPv4 and IPv6. Other protocols are free to treat numeric service names (aka port) differently.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gpshead
gpshead approved these changes Feb 14, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Feb 14, 2023
@gpshead
gpshead merged commit 928752c into python:main Feb 14, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Feb 14, 2023
@kulikjak kulikjak mentioned this pull request May 9, 2023
gh-74895: adjust tests to work on Solaris #104326
Merged
@CendioHalim CendioHalim mentioned this pull request Dec 27, 2023
sockets convert out-of-range port numbers % 2**16 #68357
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead approved these changes
+1 more reviewer
@MaxwellDupre MaxwellDupre MaxwellDupre requested changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@gpshead gpshead

Labels

extension-modules C modules in the Modules dir topic-IO type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@smejkar @arhadthedev @gpshead @MaxwellDupre @brettcannon @Mariatta @the-knights-who-say-ni @ezio-melotti @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.