From dd01087fc5b7c3f1c162288781fee889265f046c Mon Sep 17 00:00:00 2001 From: Fantix King Date: Tue, 14 Mar 2023 15:14:48 -0400 Subject: [PATCH 1/4] Update for rules of instance names * Instance names allow leading digits * Cloud instance name max length: 62 * Dashes are allowed, except for consecutive ones like -- --- edgedb/con_utils.py | 24 +++++++++++++++++------- tests/shared-client-testcases | 2 +- tests/test_con_utils.py | 2 ++ 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/edgedb/con_utils.py b/edgedb/con_utils.py index eb79f6e0c..13c3d2e2f 100644 --- a/edgedb/con_utils.py +++ b/edgedb/con_utils.py @@ -74,7 +74,12 @@ r'((?:(?:\s|^)-\s*)?\d*\.?\d*)\s*(?i:us(\s|\d|\.|$)|microseconds?(\s|$))', ) INSTANCE_NAME_RE = re.compile( - r'^([A-Za-z_]\w*)(?:/([A-Za-z_]\w*))?$', + r'^(\w(?:\w|-(?=\w))*)(?:/(\w(?:\w|-(?=\w))*))?$', + re.ASCII, +) +DSN_RE = re.compile( + r'^[a-z]+://', + re.IGNORECASE, ) @@ -519,11 +524,10 @@ def _parse_connect_dsn_and_args( ): resolved_config = ResolvedConnectConfig() - dsn, instance_name = ( - (dsn, None) - if dsn is not None and re.match('(?i)^[a-z]+://', dsn) - else (None, dsn) - ) + if dsn and DSN_RE.match(dsn): + instance_name = None + else: + instance_name, dsn = dsn, None has_compound_options = _resolve_config_options( resolved_config, @@ -861,6 +865,12 @@ def _parse_cloud_instance_name_into_config( org_slug: str, instance_name: str, ): + label = f"{instance_name}--{org_slug}" + if len(label) > 63: + raise ValueError( + f"invalid instance name: too long for cloud: " + f"{org_slug}/{instance_name}" + ) secret_key = resolved_config.secret_key if secret_key is None: try: @@ -889,7 +899,7 @@ def _parse_cloud_instance_name_into_config( raise errors.ClientConnectionError("Invalid secret key") payload = f"{org_slug}/{instance_name}".encode("utf-8") dns_bucket = binascii.crc_hqx(payload, 0) % 100 - host = f"{instance_name}--{org_slug}.c-{dns_bucket:02d}.i.{dns_zone}" + host = f"{label}.c-{dns_bucket:02d}.i.{dns_zone}" resolved_config.set_host(host, source) diff --git a/tests/shared-client-testcases b/tests/shared-client-testcases index 72675edfd..5e50beb1b 160000 --- a/tests/shared-client-testcases +++ b/tests/shared-client-testcases @@ -1 +1 @@ -Subproject commit 72675edfd43cd39bbe39b706e0847453676aac35 +Subproject commit 5e50beb1b051257a4bdc9719c3c5f38d2f6927c8 diff --git a/tests/test_con_utils.py b/tests/test_con_utils.py index 5854a76bd..742391bae 100644 --- a/tests/test_con_utils.py +++ b/tests/test_con_utils.py @@ -46,6 +46,8 @@ class TestConUtils(unittest.TestCase): RuntimeError, 'cannot read credentials'), 'invalid_dsn_or_instance_name': ( ValueError, 'invalid DSN or instance name'), + 'invalid_instance_name': ( + ValueError, 'invalid instance name'), 'invalid_dsn': (ValueError, 'invalid DSN'), 'unix_socket_unsupported': ( ValueError, 'unix socket paths not supported'), From b2d2910459a607fe4b2ccd4194159f4cb9ee448a Mon Sep 17 00:00:00 2001 From: Fantix King Date: Wed, 15 Mar 2023 12:42:10 -0400 Subject: [PATCH 2/4] CRF: address code review comments --- edgedb/con_utils.py | 8 +++++--- tests/shared-client-testcases | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/edgedb/con_utils.py b/edgedb/con_utils.py index 13c3d2e2f..becf2bcba 100644 --- a/edgedb/con_utils.py +++ b/edgedb/con_utils.py @@ -74,13 +74,14 @@ r'((?:(?:\s|^)-\s*)?\d*\.?\d*)\s*(?i:us(\s|\d|\.|$)|microseconds?(\s|$))', ) INSTANCE_NAME_RE = re.compile( - r'^(\w(?:\w|-(?=\w))*)(?:/(\w(?:\w|-(?=\w))*))?$', + r'^(\w(?:-?\w)*)(?:/(\w(?:-?\w)*))?$', re.ASCII, ) DSN_RE = re.compile( r'^[a-z]+://', re.IGNORECASE, ) +DOMAIN_LABEL_MAX_LENGTH = 63 class ClientConfiguration(typing.NamedTuple): @@ -866,9 +867,10 @@ def _parse_cloud_instance_name_into_config( instance_name: str, ): label = f"{instance_name}--{org_slug}" - if len(label) > 63: + if len(label) > DOMAIN_LABEL_MAX_LENGTH: raise ValueError( - f"invalid instance name: too long for cloud: " + f"invalid instance name: cloud instance name length cannot exceed " + f"{DOMAIN_LABEL_MAX_LENGTH - 1} characters: " f"{org_slug}/{instance_name}" ) secret_key = resolved_config.secret_key diff --git a/tests/shared-client-testcases b/tests/shared-client-testcases index 5e50beb1b..0a5c0bae7 160000 --- a/tests/shared-client-testcases +++ b/tests/shared-client-testcases @@ -1 +1 @@ -Subproject commit 5e50beb1b051257a4bdc9719c3c5f38d2f6927c8 +Subproject commit 0a5c0bae7daa38c9d47ff6eddc4897169cf51179 From f62b39aa31c11693a8ecd7e859a16dac70438abf Mon Sep 17 00:00:00 2001 From: Fantix King Date: Mon, 20 Mar 2023 18:53:08 -0400 Subject: [PATCH 3/4] update: ban underscores for the cloud --- edgedb/con_utils.py | 24 ++++++++++++++---------- tests/shared-client-testcases | 2 +- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/edgedb/con_utils.py b/edgedb/con_utils.py index becf2bcba..d8e29881f 100644 --- a/edgedb/con_utils.py +++ b/edgedb/con_utils.py @@ -74,7 +74,11 @@ r'((?:(?:\s|^)-\s*)?\d*\.?\d*)\s*(?i:us(\s|\d|\.|$)|microseconds?(\s|$))', ) INSTANCE_NAME_RE = re.compile( - r'^(\w(?:-?\w)*)(?:/(\w(?:-?\w)*))?$', + r'^(\w(?:-?\w)*)$', + re.ASCII, +) +CLOUD_INSTANCE_NAME_RE = re.compile( + r'^([A-Za-z0-9](?:-?[A-Za-z0-9])*)/([A-Za-z0-9](?:-?[A-Za-z0-9])*)$', re.ASCII, ) DSN_RE = re.compile( @@ -985,23 +989,23 @@ def _resolve_config_options( else: creds = cred_utils.validate_credentials(cred_data) source = "credentials" + elif INSTANCE_NAME_RE.match(instance_name[0]): + source = instance_name[1] + creds = cred_utils.read_credentials( + cred_utils.get_credentials_path(instance_name[0]), + ) else: - name_match = INSTANCE_NAME_RE.match(instance_name[0]) + name_match = CLOUD_INSTANCE_NAME_RE.match(instance_name[0]) if name_match is None: raise ValueError( f'invalid DSN or instance name: "{instance_name[0]}"' ) source = instance_name[1] org, inst = name_match.groups() - if inst is not None: - _parse_cloud_instance_name_into_config( - resolved_config, source, org, inst - ) - return True - - creds = cred_utils.read_credentials( - cred_utils.get_credentials_path(instance_name[0]), + _parse_cloud_instance_name_into_config( + resolved_config, source, org, inst ) + return True resolved_config.set_host(creds.get('host'), source) resolved_config.set_port(creds.get('port'), source) diff --git a/tests/shared-client-testcases b/tests/shared-client-testcases index 0a5c0bae7..40f3d513c 160000 --- a/tests/shared-client-testcases +++ b/tests/shared-client-testcases @@ -1 +1 @@ -Subproject commit 0a5c0bae7daa38c9d47ff6eddc4897169cf51179 +Subproject commit 40f3d513ce79f016480e0b22e20859fb271c2587 From 9f772924c9ed59bfe4f1921aeb9956c9e4159971 Mon Sep 17 00:00:00 2001 From: Fantix King Date: Wed, 22 Mar 2023 13:17:31 -0400 Subject: [PATCH 4/4] Update shared testcases --- tests/shared-client-testcases | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/shared-client-testcases b/tests/shared-client-testcases index 40f3d513c..32cb54cad 160000 --- a/tests/shared-client-testcases +++ b/tests/shared-client-testcases @@ -1 +1 @@ -Subproject commit 40f3d513ce79f016480e0b22e20859fb271c2587 +Subproject commit 32cb54cad0d414874962c0d31f2380b3198c07ae