This is an automated email from the ASF dual-hosted git repository.
mtaha pushed a commit to branch PG15
in repository https://gitbox.apache.org/repos/asf/age.git
The following commit(s) were added to refs/heads/PG15 by this push:
new de2fb5d3 Fix issue 1986: Failure creating label name close to
MAX_LABEL_NAME_LEN (#1993)
de2fb5d3 is described below
commit de2fb5d3162cc07518584f0bb0a1b35f6745d80d
Author: John Gemignani <[email protected]>
AuthorDate: Wed Jul 31 11:35:50 2024 -0700
Fix issue 1986: Failure creating label name close to MAX_LABEL_NAME_LEN
(#1993)
Fixed the issue with creating a label name close to MAX_LABEL_NAME_LEN.
Well,
sort of.
The issue here is that a label name is the name of a relation in PostgreSQL.
While the relation name is a char * it is basically converted to a PG type
Name
when it is used to create a relation. This conversion is a silent
truncation,
there are no warnings or error messages. The Name type is defined as
follows -
/*
* Representation of a Name: effectively just a C string, but
null-padded to
* exactly NAMEDATALEN bytes. The use of a struct is historical.
*/
typedef struct nameData
{
char data[NAMEDATALEN];
} NameData;
typedef NameData *Name;
This effectively gives us a max label name length of NAMEDATALEN - 1.
/*
* Maximum length for identifiers (e.g. table names, column names,
* function names). Names actually are limited to one fewer byte than
this,
* because the length must include a trailing zero byte.
*
* This should be at least as much as NAMEDATALEN of the database the
* applications run against.
*/
#define NAMEDATALEN 64
Since there isn't a way to get around this, the code was modified to
reflect the
usage of NAMEDATALEN for the length of label names. This required modifying
the
input parameters to be cstrings, for the functions `create_vlabel` and
`create_elabel` which allows us to tell when NAMEDATALEN has been exceeded.
Additionally, the function `is_valid_label` was renamed to
`is_valid_label_name`
for consistency.
Regression tests were updated and added.
---
age--1.5.0--y.y.y.sql | 17 ++++++
regress/expected/name_validation.out | 54 ++++++++++++++++++-
regress/sql/name_validation.sql | 22 ++++++++
sql/age_main.sql | 4 +-
src/backend/commands/label_commands.c | 98 ++++++++++++++++++-----------------
src/backend/utils/name_validation.c | 7 ++-
src/include/utils/name_validation.h | 4 +-
7 files changed, 152 insertions(+), 54 deletions(-)
diff --git a/age--1.5.0--y.y.y.sql b/age--1.5.0--y.y.y.sql
index 1620dfb4..710a196a 100644
--- a/age--1.5.0--y.y.y.sql
+++ b/age--1.5.0--y.y.y.sql
@@ -129,3 +129,20 @@ CREATE FUNCTION ag_catalog.graph_exists(graph_name name)
RETURNS agtype
LANGUAGE c
AS 'MODULE_PATHNAME', 'age_graph_exists';
+
+CREATE FUNCTION ag_catalog.age_is_valid_label_name(agtype)
+ RETURNS boolean
+ LANGUAGE c
+ IMMUTABLE
+PARALLEL SAFE
+AS 'MODULE_PATHNAME';
+
+CREATE OR REPLACE FUNCTION ag_catalog.create_vlabel(graph_name cstring,
label_name cstring)
+ RETURNS void
+ LANGUAGE c
+ AS 'MODULE_PATHNAME';
+
+CREATE OR REPLACE FUNCTION ag_catalog.create_elabel(graph_name cstring,
label_name cstring)
+ RETURNS void
+ LANGUAGE c
+ AS 'MODULE_PATHNAME';
diff --git a/regress/expected/name_validation.out
b/regress/expected/name_validation.out
index 232582bc..a9f80407 100644
--- a/regress/expected/name_validation.out
+++ b/regress/expected/name_validation.out
@@ -233,8 +233,10 @@ NOTICE: graph "graph123" has been created
-- length
-- invalid
SELECT create_vlabel('graph123', '');
+WARNING: label name length not in range (1 <= length <= 63) length = 0
ERROR: label name is invalid
SELECT create_elabel('graph123', '');
+WARNING: label name length not in range (1 <= length <= 63) length = 0
ERROR: label name is invalid
-- valid
SELECT create_vlabel('graph123', 'labelx');
@@ -396,9 +398,57 @@ SELECT * from cypher('graph123', $$ return
is_valid_label_name('label2') $$) as
true
(1 row)
+-- issue 1986: label name validation of long names.
+-- Label names are relation names which are restricted to NAMEDATALEN-1 in
size.
+-- However, we can't validate PG type Names due to namein() truncating anything
+-- over NAMEDATALEN-1. To allow the label names to be checked over
NAMEDATELEN-1
+-- we changed the input type from PG's Name to cstring. These checks are to
+-- verify that these can now be caught.
+--
+-- should return false and a warning.
+SELECT * from cypher('graph123', $$ return
is_valid_label_name('label01234567890123456789012345678901234567890123456789012345678')
$$) as (result agtype);
+WARNING: label name length not in range (1 <= length <= 63) length = 64
+ result
+--------
+ false
+(1 row)
+
+-- should be successful
+SELECT * from cypher('graph123', $$ return
is_valid_label_name('label0123456789012345678901234567890123456789012345678901234567')
$$) as (result agtype);
+ result
+--------
+ true
+(1 row)
+
+--
+-- now check vlabel creation, should fail
+SELECT create_vlabel('graph123',
'vlabel01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678');
+WARNING: label name length not in range (1 <= length <= 63) length = 95
+ERROR: label name is invalid
+-- should be successful
+SELECT create_vlabel('graph123',
'vlabel012345678901234567890123456789012345678901234567890123456');
+NOTICE: VLabel
"vlabel012345678901234567890123456789012345678901234567890123456" has been
created
+ create_vlabel
+---------------
+
+(1 row)
+
+--
+-- now check elabel creation, should fail
+SELECT create_elabel('graph123',
'elabel0123456789012345678901234567890123456789012345678901234567');
+WARNING: label name length not in range (1 <= length <= 63) length = 64
+ERROR: label name is invalid
+-- should be okay
+SELECT create_elabel('graph123',
'elabel012345678901234567890123456789012345678901234567890123456');
+NOTICE: ELabel
"elabel012345678901234567890123456789012345678901234567890123456" has been
created
+ create_elabel
+---------------
+
+(1 row)
+
-- clean up
SELECT drop_graph('graph123', true);
-NOTICE: drop cascades to 18 other objects
+NOTICE: drop cascades to 20 other objects
DETAIL: drop cascades to table graph123._ag_label_vertex
drop cascades to table graph123._ag_label_edge
drop cascades to table graph123.labelx
@@ -417,6 +467,8 @@ drop cascades to table graph123.mylabel
drop cascades to table graph123."A"
drop cascades to table graph123.mylabel2
drop cascades to table graph123."C"
+drop cascades to table
graph123.vlabel012345678901234567890123456789012345678901234567890123456
+drop cascades to table
graph123.elabel012345678901234567890123456789012345678901234567890123456
NOTICE: graph "graph123" has been dropped
drop_graph
------------
diff --git a/regress/sql/name_validation.sql b/regress/sql/name_validation.sql
index bfb5d886..0c96a25f 100644
--- a/regress/sql/name_validation.sql
+++ b/regress/sql/name_validation.sql
@@ -153,6 +153,28 @@ SELECT * from cypher('graph123', $$ return
is_valid_label_name('2label') $$) as
SELECT * from cypher('graph123', $$ return is_valid_label_name('label1') $$)
as (result agtype);
SELECT * from cypher('graph123', $$ return is_valid_label_name('label2') $$)
as (result agtype);
+-- issue 1986: label name validation of long names.
+-- Label names are relation names which are restricted to NAMEDATALEN-1 in
size.
+-- However, we can't validate PG type Names due to namein() truncating anything
+-- over NAMEDATALEN-1. To allow the label names to be checked over
NAMEDATELEN-1
+-- we changed the input type from PG's Name to cstring. These checks are to
+-- verify that these can now be caught.
+--
+-- should return false and a warning.
+SELECT * from cypher('graph123', $$ return
is_valid_label_name('label01234567890123456789012345678901234567890123456789012345678')
$$) as (result agtype);
+-- should be successful
+SELECT * from cypher('graph123', $$ return
is_valid_label_name('label0123456789012345678901234567890123456789012345678901234567')
$$) as (result agtype);
+--
+-- now check vlabel creation, should fail
+SELECT create_vlabel('graph123',
'vlabel01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678');
+-- should be successful
+SELECT create_vlabel('graph123',
'vlabel012345678901234567890123456789012345678901234567890123456');
+--
+-- now check elabel creation, should fail
+SELECT create_elabel('graph123',
'elabel0123456789012345678901234567890123456789012345678901234567');
+-- should be okay
+SELECT create_elabel('graph123',
'elabel012345678901234567890123456789012345678901234567890123456');
+
-- clean up
SELECT drop_graph('graph123', true);
diff --git a/sql/age_main.sql b/sql/age_main.sql
index 3bea3dac..59ada0f9 100644
--- a/sql/age_main.sql
+++ b/sql/age_main.sql
@@ -100,12 +100,12 @@ CREATE FUNCTION ag_catalog.drop_graph(graph_name name,
cascade boolean = false)
LANGUAGE c
AS 'MODULE_PATHNAME';
-CREATE FUNCTION ag_catalog.create_vlabel(graph_name name, label_name name)
+CREATE FUNCTION ag_catalog.create_vlabel(graph_name cstring, label_name
cstring)
RETURNS void
LANGUAGE c
AS 'MODULE_PATHNAME';
-CREATE FUNCTION ag_catalog.create_elabel(graph_name name, label_name name)
+CREATE FUNCTION ag_catalog.create_elabel(graph_name cstring, label_name
cstring)
RETURNS void
LANGUAGE c
AS 'MODULE_PATHNAME';
diff --git a/src/backend/commands/label_commands.c
b/src/backend/commands/label_commands.c
index 689dbbf6..1b2195a0 100644
--- a/src/backend/commands/label_commands.c
+++ b/src/backend/commands/label_commands.c
@@ -130,7 +130,7 @@ Datum age_is_valid_label_name(PG_FUNCTION_ARGS)
label_name = pnstrdup(agtv_value->val.string.val,
agtv_value->val.string.len);
- is_valid = is_valid_label(label_name, 0);
+ is_valid = is_valid_label_name(label_name, 0);
pfree(label_name);
if (is_valid)
@@ -155,17 +155,11 @@ PG_FUNCTION_INFO_V1(create_vlabel);
Datum create_vlabel(PG_FUNCTION_ARGS)
{
- char *graph;
- Name graph_name;
- char *graph_name_str;
+ char *graph_name;
Oid graph_oid;
List *parent;
-
RangeVar *rv;
-
- char *label;
- Name label_name;
- char *label_name_str;
+ char *label_name;
/* checking if user has not provided the graph name */
if (PG_ARGISNULL(0))
@@ -181,42 +175,49 @@ Datum create_vlabel(PG_FUNCTION_ARGS)
errmsg("label name must not be NULL")));
}
- graph_name = PG_GETARG_NAME(0);
- label_name = PG_GETARG_NAME(1);
+ graph_name = PG_GETARG_CSTRING(0);
+ label_name = PG_GETARG_CSTRING(1);
- graph_name_str = NameStr(*graph_name);
- label_name_str = NameStr(*label_name);
+ /* validate the graph and label names */
+ if (is_valid_graph_name(graph_name) == 0)
+ {
+ ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("graph name is invalid")));
+ }
+
+ if (is_valid_label_name(label_name, 0) == 0)
+ {
+ ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("label name is invalid")));
+ }
/* Check if graph does not exist */
- if (!graph_exists(graph_name_str))
+ if (!graph_exists(graph_name))
{
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_SCHEMA),
- errmsg("graph \"%s\" does not exist.",
graph_name_str)));
+ errmsg("graph \"%s\" does not exist.", graph_name)));
}
- graph_oid = get_graph_oid(graph_name_str);
+ graph_oid = get_graph_oid(graph_name);
/* Check if label with the input name already exists */
- if (label_exists(label_name_str, graph_oid))
+ if (label_exists(label_name, graph_oid))
{
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_SCHEMA),
- errmsg("label \"%s\" already exists",
label_name_str)));
+ errmsg("label \"%s\" already exists", label_name)));
}
/* Create the default label tables */
- graph = graph_name->data;
- label = label_name->data;
-
- rv = get_label_range_var(graph, graph_oid, AG_DEFAULT_LABEL_VERTEX);
+ rv = get_label_range_var(graph_name, graph_oid, AG_DEFAULT_LABEL_VERTEX);
parent = list_make1(rv);
- create_label(graph, label, LABEL_TYPE_VERTEX, parent);
+ create_label(graph_name, label_name, LABEL_TYPE_VERTEX, parent);
ereport(NOTICE,
- (errmsg("VLabel \"%s\" has been created", NameStr(*label_name))));
+ (errmsg("VLabel \"%s\" has been created", label_name)));
PG_RETURN_VOID();
}
@@ -235,17 +236,11 @@ PG_FUNCTION_INFO_V1(create_elabel);
Datum create_elabel(PG_FUNCTION_ARGS)
{
- char *graph;
- Name graph_name;
- char *graph_name_str;
+ char *graph_name;
Oid graph_oid;
List *parent;
-
RangeVar *rv;
-
- char *label;
- Name label_name;
- char *label_name_str;
+ char *label_name;
/* checking if user has not provided the graph name */
if (PG_ARGISNULL(0))
@@ -261,41 +256,48 @@ Datum create_elabel(PG_FUNCTION_ARGS)
errmsg("label name must not be NULL")));
}
- graph_name = PG_GETARG_NAME(0);
- label_name = PG_GETARG_NAME(1);
+ graph_name = PG_GETARG_CSTRING(0);
+ label_name = PG_GETARG_CSTRING(1);
- graph_name_str = NameStr(*graph_name);
- label_name_str = NameStr(*label_name);
+ /* validate the graph and label names */
+ if (is_valid_graph_name(graph_name) == 0)
+ {
+ ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("graph name is invalid")));
+ }
+
+ if (is_valid_label_name(label_name, 0) == 0)
+ {
+ ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("label name is invalid")));
+ }
/* Check if graph does not exist */
- if (!graph_exists(graph_name_str))
+ if (!graph_exists(graph_name))
{
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_SCHEMA),
- errmsg("graph \"%s\" does not exist.", graph_name_str)));
+ errmsg("graph \"%s\" does not exist.", graph_name)));
}
- graph_oid = get_graph_oid(graph_name_str);
+ graph_oid = get_graph_oid(graph_name);
/* Check if label with the input name already exists */
- if (label_exists(label_name_str, graph_oid))
+ if (label_exists(label_name, graph_oid))
{
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_SCHEMA),
- errmsg("label \"%s\" already exists",
label_name_str)));
+ errmsg("label \"%s\" already exists", label_name)));
}
/* Create the default label tables */
- graph = graph_name->data;
- label = label_name->data;
-
- rv = get_label_range_var(graph, graph_oid, AG_DEFAULT_LABEL_EDGE);
+ rv = get_label_range_var(graph_name, graph_oid, AG_DEFAULT_LABEL_EDGE);
parent = list_make1(rv);
- create_label(graph, label, LABEL_TYPE_EDGE, parent);
+ create_label(graph_name, label_name, LABEL_TYPE_EDGE, parent);
ereport(NOTICE,
- (errmsg("ELabel \"%s\" has been created", NameStr(*label_name))));
+ (errmsg("ELabel \"%s\" has been created", label_name)));
PG_RETURN_VOID();
}
@@ -318,7 +320,7 @@ void create_label(char *graph_name, char *label_name, char
label_type,
int32 label_id;
Oid relation_id;
- if (!is_valid_label(label_name, label_type))
+ if (!is_valid_label_name(label_name, label_type))
{
ereport(ERROR, (errcode(ERRCODE_UNDEFINED_SCHEMA),
errmsg("label name is invalid")));
diff --git a/src/backend/utils/name_validation.c
b/src/backend/utils/name_validation.c
index 2ee998de..6bfd62f2 100644
--- a/src/backend/utils/name_validation.c
+++ b/src/backend/utils/name_validation.c
@@ -55,12 +55,17 @@ int is_valid_graph_name(const char *graph_name)
* @param label_type label type defined in label_commands.h
* @return int
*/
-int is_valid_label(char *label_name, char label_type)
+int is_valid_label_name(char *label_name, char label_type)
{
int len = strlen(label_name);
if (len < MIN_LABEL_NAME_LEN || len > MAX_LABEL_NAME_LEN)
{
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("label name length not in range (%d <= length <= %d)
length = %d",
+ MIN_LABEL_NAME_LEN, MAX_LABEL_NAME_LEN, len)));
+
return 0;
}
diff --git a/src/include/utils/name_validation.h
b/src/include/utils/name_validation.h
index 7cfe9bf7..430da672 100644
--- a/src/include/utils/name_validation.h
+++ b/src/include/utils/name_validation.h
@@ -35,10 +35,10 @@
#define MAX_GRAPH_NAME_LEN 63
#define MIN_GRAPH_NAME_LEN 3
-#define MAX_LABEL_NAME_LEN 65535
+#define MAX_LABEL_NAME_LEN NAMEDATALEN -1
#define MIN_LABEL_NAME_LEN 1
int is_valid_graph_name(const char *graph_name);
-int is_valid_label(char *label_name, char label_type);
+int is_valid_label_name(char *label_name, char label_type);
#endif
