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

gh-110467: Fix EOF occurred in violation of protocol starting Python3.10 on large requests #115273

Closed
wants to merge 7 commits into from

Conversation

keepworking
Copy link
Contributor

@keepworking keepworking commented Feb 11, 2024

In the process of upgrading from Python 3.9 to 3.10, 'SSL_write()' was changed to 'SSL_write_ex()'.

At this time, the range of the return value of each function was different and corresponded within the function '_ssl__SSLSocket_write_impl', but the function 'PySSL_SetError' did not correspond to this.

Therefore, even if there is 'err.c' in 'SSL_ERROR_SYSCALL', the return value of 'SSL_write_ex' is always 0, so it cannot enter the condition statement.

If this commitment is approved, all of 3.10, 3.11, and 3.12 are backportable.

Copy link

cpython-cla-bot bot commented Feb 11, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Feb 11, 2024

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.

@bedevere-app
Copy link

bedevere-app bot commented Feb 11, 2024

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
Copy link
Contributor Author

#25553

Hello, @tiran I have qustion about this update of test code

When I looked at the linked pull request, I could see that the processing of ConnectionResetError has been modified to be incorporated into the lower part of OSError.

Before the correction, the server is not stopped when the error is ConnectionResetError, but after the correction, the server is also stopped when the error is ConnectionResetError.

I wonder if this is the intended correction.

except (ConnectionResetError, ConnectionAbortedError):
    # XXX: OpenSSL 1.1.1 sometimes raises ConnectionResetError
    # when connection is not shut down gracefully.
    if self.server.chatty and support.verbose:
        sys.stdout.write(
            " Connection reset by peer: {}\n".format(
                self.addr)
        )
    self.close()
    self.running = False 
    # <<<<< do not stop the server in this exception
except ssl.SSLError as err:
    # On Windows sometimes test_pha_required_nocert receives the
    # PEER_DID_NOT_RETURN_A_CERTIFICATE exception
    # before the 'tlsv13 alert certificate required' exception.
    # If the server is stopped when PEER_DID_NOT_RETURN_A_CERTIFICATE
    # is received test_pha_required_nocert fails with ConnectionResetError
    # because the underlying socket is closed
    if 'PEER_DID_NOT_RETURN_A_CERTIFICATE' == err.reason:
        if self.server.chatty and support.verbose:
            sys.stdout.write(err.args[1])
        # test_pha_required_nocert is expecting this exception
        raise ssl.SSLError('tlsv13 alert certificate required')
except OSError:
    if self.server.chatty:
        handle_error("Test server failure:\n")
    self.close()
    self.running = False

    # normally, we'd just stop here, but for the test
    # harness, we want to stop the server
    self.server.stop()
except OSError as e:
    # handles SSLError and socket errors
    if self.server.chatty and support.verbose:
        if isinstance(e, ConnectionError):
            # OpenSSL 1.1.1 sometimes raises
            # ConnectionResetError when connection is not
            # shut down gracefully.
            print(
                f" Connection reset by peer: {self.addr}"
            )
        else:
            handle_error("Test server failure:\n")
    try:
        self.write(b"ERROR\n")
    except OSError:
        pass
    self.close()
    self.running = False

    # normally, we'd just stop here, but for the test
    # harness, we want to stop the server
    self.server.stop()
    # <<<<< stop the server in this exception

@bedevere-app
Copy link

bedevere-app bot commented Feb 11, 2024

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.

@bedevere-app
Copy link

bedevere-app bot commented Feb 11, 2024

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.

@bedevere-app
Copy link

bedevere-app bot commented Feb 11, 2024

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.

…rt assertRaisesRegex (modify ConnectionResetError -> closed by the remote host)
@bedevere-app
Copy link

bedevere-app bot commented Feb 11, 2024

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
Copy link
Contributor Author

moved issue to #115627

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant