10000 gh-123954: proposal for improving logic for setting SSL exceptions by picnixz · Pull Request #128391 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

gh-123954: proposal for improving logic for setting SSL exceptions #128391

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

Draft
wants to merge 39 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
da3968c
introduce semantic types for SSL packed errors
picnixz Dec 31, 2024
704e6e3
correct error deduction in `fill_and_set_sslerror` and `_setSSLError`
picnixz Dec 31, 2024
864339a
pre-initialize default exception attributes
picnixz Dec 31, 2024
87a501d
extract logic for fetching library and reason
picnixz Dec 31, 2024
7cb16c0
extract logic for formatting error message
10000 picnixz Dec 31, 2024
4bd14a0
extract logic for building simple error message
picnixz Dec 31, 2024
c29f0f1
extract logic for building SSL verification exceptions
picnixz Dec 31, 2024
d8de992
upgrade logic for building SSL exceptions
picnixz Dec 31, 2024
ba1bf20
fix various issues
picnixz Dec 31, 2024
11ee17c
use `PyOS_snprintf` instead of `PyUnicode_FromFormat`
picnixz Jan 1, 2025
45c7a1e
fix Windows compilation
picnixz Jan 1, 2025
4362dae
Apply suggestions from code review
picnixz Jan 1, 2025
31ce5f0
Update Modules/_ssl.c
picnixz Jan 1, 2025
2fbcae2
post-merge
picnixz Jan 2, 2025
fd7d3ec
use `_Py_hashtable_t` for `err_codes_to_names`
picnixz Jan 3, 2025
7772113
use `_Py_hashtable_t` for `lib_codes_to_names`
picnixz Jan 3, 2025
4a2ed1d
post-merge
picnixz Jan 3, 2025
630c2b5
fix compilation?
picnixz Jan 3, 2025
98c8b99
attempt various things
picnixz Jan 3, 2025
133ec9f
try even more things!
picnixz Jan 3, 2025
fb5d800
:@
picnixz Jan 3, 2025
d6a0529
:@@@@@@@@@@@@@@
picnixz Jan 3, 2025
bb043b2
skip duplicated entries if they are correct
picnixz Jan 3, 2025
f836684
log more stuff
picnixz Jan 3, 2025
db3ed1c
change hash function key comparator?
picnixz Jan 4, 2025
7a198d4
revert macro
picnixz Jan 4, 2025
93a7a9b
full debug
picnixz Jan 4, 2025
95e1021
remove un-necessary specializations
picnixz Jan 4, 2025
b8f0cce
select when to use verbose SSL errors
picnixz Jan 5, 2025
689568f
reduce the number of format units
picnixz Jan 5, 2025
5583aa5
optimize `format_ssl_error_message`
picnixz Jan 5, 2025
a0ab1c2
temporarily patch CI
picnixz Jan 5, 2025
6b38769
fix assertions
picnixz Jan 5, 2025
3ecba48
now fix assertions correctly
picnixz Jan 5, 2025
d7f1ab1
use ImportWarning insteaad of RuntimeWarning
picnixz Jan 5, 2025
32f7cba
clear exception after emitting the warning
picnixz Jan 5, 2025
a88ec91
fix error path when formatting message
picnixz Jan 9, 2025
a9a6a50
Merge remote-tracking branch 'upstream/main' into perf/ssl/error-123954
picnixz Jan 11, 2025
68f8bc7
include hexadecimal error code value when reporting unknown error
picnixz Jan 11, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
optimize format_ssl_error_message
  • Loading branch information
picnixz committed Jan 5, 2025
commit 5583aa54f40d06e30f524a7a4f1da7c0030ad896
148 changes: 92 additions & 56 deletions Modules/_ssl.c
10000
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,40 @@ ssl_error_fetch_lib_and_reason(_sslmodulestate *state, py_ssl_errcode errcode,
*reason = Py_XNewRef(val);
}

static inline size_t
ssl_error_filename_width(const char *Py_UNUSED(filename))
{
/*
* Sometimes, __FILE__ is an absolute path, so we hardcode "_ssl.c".
* In the future, we might want to use the "filename" parameter but
* for now (and for backward compatibility), we ignore it.
*/
return 6 /* strlen("_ssl.c") */;
}

static inline size_t
ssl_error_lineno_width(int lineno)
{
if (lineno < 0) {
return 1 + ssl_error_lineno_width(-lineno);
}
#define FAST_PATH(E, N) \
do { \
assert((size_t)(1e ## E) == N); \
if (lineno < (N)) { \
return (E); \
} \
} while (0)
FAST_PATH(2, 10);
FAST_PATH(3, 100);
FAST_PATH(4, 1000);
FAST_PATH(5, 10000);
FAST_PATH(6, 100000);
FAST_PATH(7, 1000000);
#undef FAST_PATH
return (size_t)ceil(log10(lineno));
}

/*
* Construct a Unicode object containing the formatted SSL error message.
*
Expand All @@ -537,7 +571,7 @@ ssl_error_fetch_lib_and_reason(_sslmodulestate *state, py_ssl_errcode errcode,
static PyObject *
format_ssl_error_message(PyObject *lib, PyObject *reason, PyObject *verify,
const char *errstr,
const char *Py_UNUSED(filename), int lineno)
const char *filename, int lineno)
{
assert(errstr != NULL);

Expand All @@ -551,71 +585,71 @@ format_ssl_error_message(PyObject *lib, PyObject *reason, PyObject *verify,
CHECK_OBJECT(verify);
#undef CHECK_OBJECT

#define OPTIONAL_UTF8(x) ((x) == NULL ? NULL : PyUnicode_AsUTF8((x)))
const char *libstr = OPTIONAL_UTF8(lib);
const char *reastr = OPTIONAL_UTF8(reason);
const char *verstr = OPTIONAL_UTF8(verify);
#undef OPTIONAL_UTF8

const size_t errstr_len = strlen(errstr);
const size_t libstr_len = libstr == NULL ? 0 : strlen(libstr);
const size_t reastr_len = reastr == NULL ? 0 : strlen(reastr);
const size_t verstr_len = verstr == NULL ? 0 : strlen(verstr);
/*
* Sometimes, __FILE__ is an absolute path, so we hardcode "_ssl.c".
* In the future, we might want to use the "filename" parameter but
* for now (and for backward compatibility), we ignore it.
*/
const size_t filename_len = 6; /* strlen("_ssl.c") */
/*
* Crude upper bound on the number of characters taken by the line number.
* We expect -1 <= lineno < 1e8. More lines are unlikely to happen.
*/
assert(lineno >= -1);
assert(lineno < 1e8);
const size_t lineno_len = 8;
const size_t base_alloc = (
libstr_len + reastr_len + verstr_len
+ errstr_len + filename_len + lineno_len
);
const size_t filename_len = ssl_error_filename_width(filename);
const size_t lineno_len = ssl_error_lineno_width(lineno);
/* exact length of "(_ssl.c:LINENO)" */
const size_t suffix_len = 1 + filename_len + 1 + lineno_len + 1;

int rc;
char *buf;

PyObject *res = NULL;
#define CSTRBUF(x) ((const char *)PyUnicode_DATA((x)))
#define CHARBUF(x) ((char *)PyUnicode_DATA((x)))
if (lib && reason && verify) {
/* [LIB: REASON] ERROR: VERIFY (_ssl.c:LINENO) */
const size_t alloc = base_alloc + (4 + 3 + 4);
/* PyMem_Malloc() is optimized for small buffers */
buf = PyMem_New(char, alloc);
rc = PyOS_snprintf(buf, alloc, "[%s: %s] %s: %s (_ssl.c:%d)",
libstr, reastr, errstr, verstr, lineno);
const char *lib_cstr = CSTRBUF(lib);
const char *reason_cstr = CSTRBUF(reason);
const char *verify_cstr = CSTRBUF(verify);
const size_t ressize = /* excludes final null byte */ (
1 + strlen(lib_cstr) + 2 + strlen(reason_cstr) + 1
+ 1 + strlen(errstr) + 2 + strlen(verify_cstr)
+ 1 + suffix_len
);
res = PyUnicode_New(ressize, 127);
rc = snprintf(CHARBUF(res), ressize + 1, "[%s: %s] %s: %s (_ssl.c:%d)",
lib_cstr, reason_cstr, errstr, verify_cstr, lineno);
}
else if (lib && reason) {
/* [LIB: REASON] ERROR (_ssl.c:LINENO) */
const size_t alloc = base_alloc + (3 + 2 + 4);
buf = PyMem_New(char, alloc);
rc = PyOS_snprintf(buf, alloc, "[%s: %s] %s (_ssl.c:%d)",
libstr, reastr, errstr, lineno);
const char *lib_cstr = CSTRBUF(lib);
const char *reason_cstr = CSTRBUF(reason);
const size_t ressize = /* excludes final null byte */ (
1 + strlen(lib_cstr) + 2 + strlen(reason_cstr) + 1
+ 1 + strlen(errstr)
+ 1 + suffix_len
);
res = PyUnicode_New(ressize, 127);
rc = snprintf(CHARBUF(res), ressize + 1, "[%s: %s] %s (_ssl.c:%d)",
lib_cstr, reason_cstr, errstr, lineno);
}
else if (lib) {
/* [LIB] ERROR (_ssl.c:LINENO) */
const size_t alloc = base_alloc + (2 + 1 + 4);
buf = PyMem_New(char, alloc);
rc = PyOS_snprintf(buf, alloc, "[%s] %s (_ssl.c:%d)",
libstr, errstr, lineno);
const char *lib_cstr = CSTRBUF(lib);
const size_t ressize = /* excludes final null byte */ (
1 + strlen(lib_cstr) + 1
+ 1 + strlen(errstr)
+ 1 + suffix_len
);
res = PyUnicode_New(ressize, 127);
rc = snprintf(CHARBUF(res), ressize + 1, "[%s] %s (_ssl.c:%d)",
lib_cstr, errstr, lineno);
}
else {
/* ERROR (_ssl.c:LINENO) */
const size_t alloc = base_alloc + (1 + 1 + 2);
buf = PyMem_New(char, alloc);
rc = PyOS_snprintf(buf, alloc, "%s (_ssl.c:%d)",
errstr, lineno);
const size_t ressize = /* excludes final null byte */ (
strlen(errstr)
+ 1 + suffix_len
);
res = PyUnicode_New(ressize, 127);
rc = snprintf(CHARBUF(res), ressize + 1, "%s (_ssl.c:%d)",
errstr, lineno);
}
#undef CHARBUF
#undef CSTRBUF
if (rc < 0) {
Py_XDECREF(res);
/* fallback to slow path if snprintf() failed */
return PyUnicode_FromFormat("%s (_ssl.c:%d)", errstr, lineno);
}

PyObject *res = rc < 0 /* fallback to slow path if snprintf() failed */
? PyUnicode_FromFormat("%s (_ssl.c:%d)", errstr, lineno)
: PyUnicode_FromString(buf) /* uses the ASCII fast path */;
PyMem_Free(buf);
return res;
}

Expand All @@ -628,7 +662,7 @@ format_ssl_error_message(PyObject *lib, PyObject *reason, PyObject *verify,
* ssl_errno The SSL error number to pass to the exception constructor.
* lib The ASCII-encoded library obtained from a packed error code.
* reason The ASCII-encoded reason obtained from a packed error code.
* errstr The error message to use.
* errstr The non-NULL error message to use.
*
* A non-NULL library or reason is stored in the final exception object.
*/
Expand Down Expand Up @@ -778,10 +812,12 @@ fill_and_set_sslerror(_sslmodulestate *state,
if (ssl_use_verbose_error(state, exc_type, ssl_errno, errcode)) {
ssl_error_fetch_lib_and_reason(state, errcode, &lib, &reason);
}
if (errstr == NULL && errcode) {
errstr = ERR_reason_error_string(errcode);
}
if (errstr == NULL) {
errstr = errcode
? ERR_reason_error_string(errcode)
: "unknown error";
// ERR_reason_error_string() may return NULL
errstr = "unknown error";
}
PyObject *exc;
if (sslsock != NULL && exc_type == state->PySSLCertVerificationErrorObject) {
Expand Down
0