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.

Sign up for GitHub

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

PYTHON-5014 Fix handling of async socket errors in kms request #2054

Merged
merged 6 commits into from
Jan 10, 2025
+32 −10
Merged

PYTHON-5014 Fix handling of async socket errors in kms request #2054

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0c0a7eb
test against older drivers-tools
blink1073 Jan 9, 2025
d696857
try with pymongocrypt 1.11
blink1073 Jan 9, 2025
8070b8d
PYTHON-5014 Fix handling of async socket errors in kms request
blink1073 Jan 10, 2025
4f007c6
undo requirements change
blink1073 Jan 10, 2025
8232f26
Address rhel error
blink1073 Jan 10, 2025
321b3e3
address review
blink1073 Jan 10, 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
3 changes: 3 additions & 0 deletions pymongo/asynchronous/encryption.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,9 @@ async def kms_request(self, kms_context: MongoCryptKmsContext) -> None:
if not data:
raise OSError("KMS connection closed")
kms_context.feed(data)
# Async raises an OSError instead of returning empty bytes
except OSError as err:
raise OSError("KMS connection closed") from err
Copy link
Member

@ShaneHarvey ShaneHarvey Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Doesn't OSError already get handled by the except Exception as exc block? It should hit the _raise_connection_failure line which adds important info to the error message like the host/port/connection timeouts.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

except MongoCryptError:
raise # Propagate MongoCryptError errors directly.
except Exception as exc:
Expand Down
3 changes: 3 additions & 0 deletions pymongo/synchronous/encryption.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,9 @@ def kms_request(self, kms_context: MongoCryptKmsContext) -> None:
if not data:
raise OSError("KMS connection closed")
kms_context.feed(data)
# Async raises an OSError instead of returning empty bytes
except OSError as err:
raise OSError("KMS connection closed") from err
except MongoCryptError:
raise # Propagate MongoCryptError errors directly.
except Exception as exc:
Expand Down
12 changes: 8 additions & 4 deletions test/asynchronous/test_encryption.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2163,7 +2163,8 @@ async def test_01_aws(self):
# 127.0.0.1:9001: ('Certificate does not contain any `subjectAltName`s.',)
key["endpoint"] = "127.0.0.1:9001"
with self.assertRaisesRegex(
EncryptionError, "IP address mismatch|wronghost|IPAddressMismatch|Certificate"
EncryptionError,
"IP address mismatch|wronghost|IPAddressMismatch|Certificate|SSL handshake failed",
):
await self.client_encryption_invalid_hostname.create_data_key("aws", key)

Expand All @@ -2180,7 +2181,8 @@ async def test_02_azure(self):
await self.client_encryption_expired.create_data_key("azure", key)
# Invalid cert hostname error.
with self.assertRaisesRegex(
EncryptionError, "IP address mismatch|wronghost|IPAddressMismatch|Certificate"
EncryptionError,
"IP address mismatch|wronghost|IPAddressMismatch|Certificate|SSL handshake failed",
):
await self.client_encryption_invalid_hostname.create_data_key("azure", key)

Expand All @@ -2197,7 +2199,8 @@ async def test_03_gcp(self):
await self.client_encryption_expired.create_data_key("gcp", key)
# Invalid cert hostname error.
with self.assertRaisesRegex(
EncryptionError, "IP address mismatch|wronghost|IPAddressMismatch|Certificate"
EncryptionError,
"IP address mismatch|wronghost|IPAddressMismatch|Certificate|SSL handshake failed",
):
await self.client_encryption_invalid_hostname.create_data_key("gcp", key)

Expand All @@ -2211,7 +2214,8 @@ async def test_04_kmip(self):
await self.client_encryption_expired.create_data_key("kmip")
# Invalid cert hostname error.
with self.assertRaisesRegex(
EncryptionError, "IP address mismatch|wronghost|IPAddressMismatch|Certificate"
EncryptionError,
"IP address mismatch|wronghost|IPAddressMismatch|Certificate|SSL handshake failed",
):
await self.client_encryption_invalid_hostname.create_data_key("kmip")

Expand Down
12 changes: 8 additions & 4 deletions test/test_encryption.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2155,7 +2155,8 @@ def test_01_aws(self):
# 127.0.0.1:9001: ('Certificate does not contain any `subjectAltName`s.',)
key["endpoint"] = "127.0.0.1:9001"
with self.assertRaisesRegex(
EncryptionError, "IP address mismatch|wronghost|IPAddressMismatch|Certificate"
EncryptionError,
"IP address mismatch|wronghost|IPAddressMismatch|Certificate|SSL handshake failed",
):
self.client_encryption_invalid_hostname.create_data_key("aws", key)

Expand All @@ -2172,7 +2173,8 @@ def test_02_azure(self):
self.client_encryption_expired.create_data_key("azure", key)
# Invalid cert hostname error.
with self.assertRaisesRegex(
EncryptionError, "IP address mismatch|wronghost|IPAddressMismatch|Certificate"
EncryptionError,
"IP address mismatch|wronghost|IPAddressMismatch|Certificate|SSL handshake failed",
):
self.client_encryption_invalid_hostname.create_data_key("azure", key)

Expand All @@ -2189,7 +2191,8 @@ def test_03_gcp(self):
self.client_encryption_expired.create_data_key("gcp", key)
# Invalid cert hostname error.
with self.assertRaisesRegex(
EncryptionError, "IP address mismatch|wronghost|IPAddressMismatch|Certificate"
EncryptionError,
"IP address mismatch|wronghost|IPAddressMismatch|Certificate|SSL handshake failed",
):
self.client_encryption_invalid_hostname.create_data_key("gcp", key)

Expand All @@ -2203,7 +2206,8 @@ def test_04_kmip(self):
self.client_encryption_expired.create_data_key("kmip")
# Invalid cert hostname error.
with self.assertRaisesRegex(
EncryptionError, "IP address mismatch|wronghost|IPAddressMismatch|Certificate"
EncryptionError,
"IP address mismatch|wronghost|IPAddressMismatch|Certificate|SSL handshake failed",
):
self.client_encryption_invalid_hostname.create_data_key("kmip")

Expand Down
Loading