gh-107545: Fix misleading setsockopt error message #107546

naweiss · 2023-08-01T19:09:27Z

According to Python's documentation, setsockopt can get either int, buffer or None as the third argument.
The way it is currently implemented is by trying to parse all of the three arguments as ints, if this fails no matter what the reason is we just try the next overload.
Since 2**31 causes PyExc_OverflowError we try the next overloads. Because the buffer overload is last we get type error for the third argument instead of OverflowError (e.g. TypeError: a bytes-like object is required, not 'int').

Issue: misleading setsockopt error message #107545

bedevere-bot · 2023-08-01T19:09:37Z

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

ghost · 2023-08-01T19:09:56Z

All commit authors signed the Contributor License Agreement.

naweiss · 2023-08-01T19:14:14Z

I choose to solve the issue using a white-list approach rather then a black-list approach since there might be more errors that PyErr_Clear will silently suppress. For example, currently, if the type of the first argument is incorrect you still need to keep checking all of the overloads of setsockopt they will all fail and you get the error message from the buffer overload. when you could report the error immediately after the first overload failure.

Error message for:

import socket with socket.socket() as s: s.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, 2 ** 31)

is now OverflowError: signed integer is greater than maximum

CharlieZhao95 · 2023-08-02T06:21:08Z

Looks good, how about adding some tests for new error messages.

Misc/NEWS.d/next/Core and Builtins/2023-08-01-22-30-35.gh-issue-107545.f9i3pQ.rst

naweiss · 2023-08-06T04:02:15Z

I added the relevant tests. Any new review comments?

CharlieZhao95 · 2023-08-07T02:09:39Z

I added the relevant tests. Any new review comments?

Considering this is your first PR in cpython, please be patient! The next step is waiting for the review by core devs.

Generally, core developers don't have enough time to review every PR, so you can see there are many PRs that can't be merged into the main. Before they come to review, we just need to keep our PR compliant and wait :)

python-cla-bot · 2025-04-18T09:41:30Z

All commit authors signed the Contributor License Agreement.

naweiss · 2025-06-25T10:15:39Z

Since this PR is old, I validated that the issue is still reproducible.
Than rebased the branch onto main, resolved any conflicts, and confirmed that all tests pass.
I also did some minor changes to make the PR files a little cleaner and follow conventions better.

serhiy-storchaka

LGTM, but I think that we can get rid of _testcapi.

The more user friendly approach is to parse arguments as "iiOO|O" and then use == Py_None, PyIndex_Check() and PyBuffer_Check() for the third argument. This will allow to generate better error messages like "... should be integer, bytes-like object or None".

serhiy-storchaka · 2025-07-07T11:48:27Z

Lib/test/test_socket.py

+ @unittest.skipIf(_testcapi is None, "requires _testcapi")
+ def test_setsockopt_errors(self):
+ # See issue #107546.
+ from _testcapi import INT_MAX, INT_MIN


We could just use some large values like 2**1000 and -2**1000.

I'm scared by big numbers, 2**100 and -2**100 should be enough to trigger OverflowError on all platforms :-)

But not on platforms with 128-bit int. I often use 2**1000 as more future proof option.

Does Python support any platform with 128-bit integer? Does such platform exist?

naweiss · 2025-07-07T17:20:04Z

Thanks for the feedback.
I would be happy for guidance regarding the AF_VSOCK family.
The old code supported only an integer option, is this the expected behavior?

serhiy-storchaka · 2025-07-07T17:25:50Z

Modules/socketmodule.c

@@ -3338,26 +3338,32 @@ sock_setsockopt(PyObject *self, PyObject *args)
 Py_buffer optval;
 int flag;
 unsigned int optlen;
- PyObject *none;
+ PyObject *type;


Semantically, this is not type, but value.

I choose the name optval for all cases which access the value of the option.

serhiy-storchaka · 2025-07-07T17:26:37Z

Modules/socketmodule.c

- PyObject *none;
+ PyObject *type;
+
+ if (!PyArg_ParseTuple(args, "iiO|I:setsockopt",


The fourth argument is only accepted if the third argument is None. One way to verify this is to use the "O" converter with a default value of NULL and check that it is still NULL in all cases except when the third argument is None.

I ended up using a different approach, since I didn't wanted the code to break in the future if someone will add another overload and forget to check that the value is NULL.
So, I first check that the code receives either 3 or 4 arguments with the appropriate types while allowing a 4th argument only if the 3rd is None.

Yes, this would work too. The minor downside of this approach is that you needs to explicitly raise an error message for wrong number of arguments.

You can also use a single PyArg_ParseTuple() with format "iiO|I:setsockopt" and then check the number of arguments:

if (arglen == 3 && optval == Py_None) { // error } if (arglen == 4 && optval != Py_None) { // error }

Bot note that only integers are currently supported for AF_VSOCK (I think we will change this, but in a separate issue).

serhiy-storchaka · 2025-07-07T17:32:43Z

Lib/test/test_socket.py

+ with self.assertRaises(OverflowError):
+ sock.setsockopt(2 ** 100, socket.SO_REUSEADDR, 1)
+
+ with self.assertRaises(TypeError):


You can test the error message as well.

Good Idea, I also created some more type checks and added their exact message to the tests code.

serhiy-storchaka · 2025-07-07T17:38:35Z

Thank you for your update, @naweiss. Here is the next portion of comments.

serhiy-storchaka · 2025-07-07T17:41:44Z

The old code supported only an integer option, is this the expected behavior?

I noticed this too. We need to look at the history of the code. Anyway, this is a different issue.

vstinner · 2025-07-08T10:27:37Z

Modules/socketmodule.c

+ if (!PyArg_ParseTuple(args, "iiO:setsockopt",
+ &level, &optname, &optval)) {
+ return NULL;
+ }


Can you please move this test for None and 4 arguments here?

if (optval == Py_None) { PyErr_Format(...) }

vstinner · 2025-07-08T10:27:49Z

Modules/socketmodule.c

+ }
+ break;
+ case 4:
+ if (!PyArg_ParseTuple(args, "iiO!I:setsockopt",


Suggested change

if (!PyArg_ParseTuple(args, "iiO!I:setsockopt",

if (!PyArg_ParseTuple(args, "iiOI:setsockopt",

vstinner · 2025-07-08T10:29:35Z

Modules/socketmodule.c

@@ -3374,36 +3397,45 @@ sock_setsockopt(PyObject *self, PyObject *args)
 goto done;
 }

- PyErr_Clear();
 /* setsockopt(level, opt, None, flag) */


Suggested change

/* setsockopt(level, opt, None, flag) */

/* setsockopt(level, opt, None, optlen) */

vstinner · 2025-07-08T10:30:21Z

Modules/socketmodule.c

+  res = setsockopt(get_sock_fd(s), level, optname,
+  buffer.buf, (int)buffer.len);


Suggested change

res = setsockopt(get_sock_fd(s), level, optname,

buffer.buf, (int)buffer.len);

res = setsockopt(get_sock_fd(s), level, optname,

buffer.buf, (int)buffer.len);

serhiy-storchaka · 2025-07-08T12:37:55Z

Modules/socketmodule.c

+ &level, &optname, Py_TYPE(Py_None), &optval, &optlen)) {
+ return NULL;
+ }
+ break;


We need if (optval != Py_None) {error...} here.

serhiy-storchaka · 2025-07-08T12:42:14Z

Lib/test/test_socket.py

+ sock.setsockopt(2 ** 100, socket.SO_REUSEADDR, 1)
+
+ msg = "socket option should be should be integer, bytes-like object or None"
+ with self.assertRaises(TypeError, msg=msg):


It should be assertRaisesRegex. And the expected error message is a regular expression, so you need to escape parentheses.

serhiy-storchaka · 2025-07-08T12:49:34Z

Modules/socketmodule.c

- PyObject *none;
+ PyObject *type;
+
+ if (!PyArg_ParseTuple(args, "iiO|I:setsockopt",


Yes, this would work too. The minor downside of this approach is that you needs to explicitly raise an error message for wrong number of arguments.

You can also use a single PyArg_ParseTuple() with format "iiO|I:setsockopt" and then check the number of arguments:

if (arglen == 3 && optval == Py_None) { // error } if (arglen == 4 && optval != Py_None) { // error }

Bot note that only integers are currently supported for AF_VSOCK (I think we will change this, but in a separate issue).

bedevere-bot mentioned this pull request Aug 1, 2023

misleading setsockopt error message #107545

Open

bedevere-bot added the awaiting review label Aug 1, 2023

CharlieZhao95 reviewed Aug 2, 2023

View reviewed changes

Misc/NEWS.d/next/Core and Builtins/2023-08-01-22-30-35.gh-issue-107545.f9i3pQ.rst Outdated Show resolved Hide resolved

naweiss added 2 commits June 23, 2025 23:50

pythongh-107545: Fix misleading setsockopt error message

f85c7dc

Add tests for setsockopt errors

82be245

naweiss force-pushed the fix-issue-107545 branch from f712b65 to 6439014 Compare June 23, 2025 21:12

Add issue to news

167d212

naweiss force-pushed the fix-issue-107545 branch from 6439014 to 167d212 Compare June 23, 2025 21:22

serhiy-storchaka reviewed Jul 7, 2025

View reviewed changes

serhiy-storchaka added the needs backport to 3.14 bugs and security fixes label Jul 7, 2025

Cleanup setsockopt implementation

e3d4e74

serhiy-storchaka reviewed Jul 7, 2025

View reviewed changes

Add clearer error messages to setsockopt

ca87487

vstinner reviewed Jul 8, 2025

View reviewed changes

serhiy-storchaka reviewed Jul 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

gh-107545: Fix misleading setsockopt error message #107546

gh-107545: Fix misleading setsockopt error message #107546

naweiss commented Aug 1, 2023 •

edited

Loading

bedevere-bot commented Aug 1, 2023

ghost commented Aug 1, 2023 •

edited by ghost

Loading

naweiss commented Aug 1, 2023 •

edited

Loading

CharlieZhao95 commented Aug 2, 2023

Uh oh!

naweiss commented Aug 6, 2023

CharlieZhao95 commented Aug 7, 2023 •

edited

Loading

python-cla-bot bot commented Apr 18, 2025 •

edited

Loading

naweiss commented Jun 25, 2025

serhiy-storchaka left a comment

serhiy-storchaka Jul 7, 2025

vstinner Jul 7, 2025

serhiy-storchaka Jul 7, 2025

vstinner Jul 7, 2025

naweiss commented Jul 7, 2025

serhiy-storchaka Jul 7, 2025

naweiss Jul 8, 2025 •

edited

Loading

serhiy-storchaka Jul 7, 2025

naweiss Jul 8, 2025

serhiy-storchaka Jul 8, 2025

serhiy-storchaka Jul 7, 2025

naweiss Jul 8, 2025

serhiy-storchaka commented Jul 7, 2025

serhiy-storchaka commented Jul 7, 2025

vstinner Jul 8, 2025

vstinner Jul 8, 2025

vstinner Jul 8, 2025

vstinner Jul 8, 2025

serhiy-storchaka Jul 8, 2025

serhiy-storchaka Jul 8, 2025

serhiy-storchaka Jul 8, 2025

	if (!PyArg_ParseTuple(args, "iiO!I:setsockopt",
	if (!PyArg_ParseTuple(args, "iiOI:setsockopt",

	/* setsockopt(level, opt, None, flag) */
	/* setsockopt(level, opt, None, optlen) */

		res = setsockopt(get_sock_fd(s), level, optname,
		buffer.buf, (int)buffer.len);

Uh oh!

gh-107545: Fix misleading setsockopt error message #107546

Are you sure you want to change the base?

gh-107545: Fix misleading setsockopt error message #107546

Conversation

naweiss commented Aug 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

bedevere-bot commented Aug 1, 2023

ghost commented Aug 1, 2023 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

naweiss commented Aug 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CharlieZhao95 commented Aug 2, 2023

Uh oh!

naweiss commented Aug 6, 2023

CharlieZhao95 commented Aug 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

python-cla-bot bot commented Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

naweiss commented Jun 25, 2025

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naweiss commented Jul 7, 2025

Choose a reason for hiding this comment

naweiss Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhiy-storchaka commented Jul 7, 2025

serhiy-storchaka commented Jul 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naweiss commented Aug 1, 2023 •

edited

Loading

ghost commented Aug 1, 2023 •

edited by ghost

Loading

naweiss commented Aug 1, 2023 •

edited

Loading

CharlieZhao95 commented Aug 7, 2023 •

edited

Loading

python-cla-bot bot commented Apr 18, 2025 •

edited

Loading

naweiss Jul 8, 2025 •

edited

Loading