gh-115627: Fix PySSL_SetError handling SSL_ERROR_SYSCALL

[keepworking]

The SSL_write() and SSL_read() used in python 3.9 were modified from python 3.10 to SSL_write_ex() and SSL_read_ex(), and the return value was fixed to 0 on error.

I modified it so that OSError can be returned correctly, not EOF, when retval is 0.

• Issue: gh-115627

[bedevere-app[bot]]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[keepworking]

Additional changes were made in the error handling logic of the code.

After correcting the condition according to the condition of socketObject as advised, we checked the error for OpenSSL 1.1.1.

Error in the following test case. test.test_ssl.SimpleBackgroundTests.test_transport_eof

The local development environment was not opensl 1.1.1 and could not be verified in advance. When I enabled and tested opensl 1.1.1 for the local environment, it was in the following conditions.

err.ssl = 5 (SSL_ERROR_SYSCALL) err.c = 0 socketObject = NULL

In the previous code, ret was 0, so it seems that the case was able to move over flexibly.

https://github.com/openssl/openssl/blob/master/doc/man3/SSL_get_error.pod#description https://www.openssl.org/docs/man1.1.1/man3/SSL_get_error.html

On an unexpected EOF, versions before OpenSSL 3.0 returned SSL_ERROR_SYSCALL, nothing was added to the error stack, and errno was 0.

The SSL_ERROR_SYSCALL with errno value of 0 indicates unexpected EOF from the peer.

In this case, it is more appropriate to rely on err.ssl and err.c, which provide accurate information about errors, rather than looking at the ret or socketObject status, because it is consistent with the EOF occurrence conditions shown in the openingssl document.

@serhiy-storchaka Thanks to a lot of advice, I was able to check more diverse test cases and trimmed the code correctly.

[encukou]

The ssl experts are: @jackjansen, @tiran, @dstufft, @alex. Do y'all have any comments? (If the ping bothers you, adjust the list, or just reply -- I can remove you or mark you inactive.)

Meanwhile, I'll read up on the context here and review as best as I can.

[keepworking]

When I tried to correct the conflict with the main branch today, I adopted the merge method, not the rebase, so the commit order was mixed up. To correct that, I pulled the main locally and rebase the working branch, but I realized that I didn't sync between the forked repo and the original. I made several mistakes because I wasn't good at it...

Nevertheless, thank you @encukou for kindly reviewing the code. CI error seems to have been corrected.

[encukou]

Please avoid rebasing and force-pushing pull requests to CPython. The PR will get squashed, so you don't need to keep the individual commits clean; changes since a previous version of the PR are much more important. With a rebase, the commit I looked at previously got lost.

Thankfully I found the previous version in the local history, and was able to compare to the new version :)

Looks good, let's merge if buildbots don't find an issue.

[bedevere-bot]

:robot: New build scheduled with the buildbot fleet by @encukou for commit 1a304667cfbd105a71c3d608780a558e9bc75d01 :robot:

If you want to schedule another build, you need to add the :hammer: test-with-buildbots label again.

[keepworking]

Thank you @encukou, I'll be careful regarding Merge. Some errors have been identified in the buildbot, which seems to be related to forking or multiprocessing. It looks like the same error appears in other PR. Please let me know if there is anything else I can do to help.

[encukou]

None of those errors look related :)

Thank you for the fix!

[gpshead]

Thanks for authoring, reviewing, and merging. I agree with @encukou's analysis.