Skip to content

Commit feefc5c

Browse files
ppisarppisar
committed
Lessen quoting for git commit IDs
Commit IDs in ref fields starting with a digit were unnecesarily quoted (ref: "26ca0c0"). This patch augments detecting numbers to hopefully anything what YAML 1.2.2 considers an integer or a float. That enables only quoting values which indeed can be interpreted as numbers and thus are endangered of loosing characters insignificant for numbers (1.20 -> 1.2). This adds also an extensive tests to what a number-like string looks like.
1 parent 6a46125 commit feefc5c

16 files changed

+252
-31
lines changed

‎modulemd/meson.build‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ c_tests = {
363363
'translation' : [ 'tests/test-modulemd-translation.c' ],
364364
'translation_entry' : [ 'tests/test-modulemd-translation-entry.c' ],
365365
'obsoletes' : [ 'tests/test-modulemd-obsoletes.c' ],
366+
'quoting' : [ 'tests/test-modulemd-quoting.c' ],
366367
}
367368

368369
foreach name, sources : c_tests

‎modulemd/modulemd-yaml-util.c‎

Lines changed: 82 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -313,29 +313,99 @@ mmd_emitter_scalar (yaml_emitter_t *emitter,
313313
* mmd_string_is_empty_or_a_number
314314
* @string (in) (nullable)
315315
*
316-
* Returns: True if the @string is %NULL, empty, or looks like a decimal
317-
* number. False otherwise (looks like a string).
316+
* A purpose is to catch the strings which could be mistaken with a number.
318317
*
319-
* Note: This function is intentionally naive (i.e. it checks only the
320-
* first character). It's fast. It does not dispute whether "0x01" is or is
321-
* not a number. A purpose is to catch strings which could be mistaken with
322-
* a number.
318+
* Returns: True if the @string is %NULL, empty, or looks like a number.
319+
* False otherwise (looks like a string).
323320
*
324321
* Since 2.15
325322
*/
326323
static gboolean
327324
string_is_empty_or_a_number (const gchar *string)
328325
{
326+
int fractional = 0;
327+
int exponent = 0;
328+
int hexadecimal = 0;
329+
int octal = 0;
330+
329331
if (string == NULL)
330332
return TRUE;
331333
if (string[0] == '\0')
332334
return TRUE;
333-
if (string[0] >= '0' && string[0] <= '9')
334-
return TRUE;
335-
if ((string[0] == '+' || string[0] == '-' || string[0] == '.') &&
336-
(string[1] >= '0' && string[1] <= '9'))
337-
return TRUE;
338-
return FALSE;
335+
336+
if (!g_strcmp0 (string, ".nan"))
337+
return TRUE; /* not a number symbol */
338+
if (string[0] == '+' || string[0] == '-')
339+
string++; /* accept a leading sign */
340+
if (!g_strcmp0 (string, ".inf"))
341+
return TRUE; /* infinity symbol */
342+
if (string[0] == '.')
343+
{
344+
fractional = 1; /* accept a leading decimal point */
345+
string++;
346+
}
347+
else if (string[0] == '0' && string[1] == 'x')
348+
{
349+
hexadecimal = 1;
350+
string += 2;
351+
}
352+
else if (string[0] == '0' && string[1] == 'o')
353+
{
354+
octal = 1;
355+
string += 2;
356+
}
357+
if (string[0] == '\0')
358+
return FALSE; /* incomplete notation */
359+
360+
for (const gchar *character = string; *character != '\0'; character++)
361+
{
362+
if (hexadecimal)
363+
{
364+
if (!((*character >= '0' && *character <= '9') ||
365+
(*character >= 'a' && *character <= 'f') ||
366+
(*character >= 'A' && *character <= 'F')))
367+
return FALSE;
368+
continue;
369+
}
370+
else if (octal)
371+
{
372+
if (!(*character >= '0' && *character <= '7'))
373+
return FALSE;
374+
continue;
375+
}
376+
else /* decimal */
377+
{
378+
if (*character == '.')
379+
{
380+
if (fractional)
381+
return FALSE; /* multiple decimal points */
382+
else
383+
{
384+
fractional = 1;
385+
continue;
386+
}
387+
}
388+
if (fractional && (*character == 'e' || *character == 'E'))
389+
{
390+
if (exponent)
391+
return FALSE; /* multiple franctional exponent */
392+
else
393+
{
394+
exponent = 1;
395+
continue;
396+
}
397+
}
398+
if (exponent == 1 && (*character == '+' || *character == '-'))
399+
{
400+
exponent = 2; /* a sign of the exponent */
401+
continue;
402+
}
403+
if (*character < '0' || *character > '9')
404+
return FALSE;
405+
}
406+
}
407+
408+
return TRUE;
339409
}
340410

341411

‎modulemd/tests/ModulemdTests/modulestream.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,7 @@ def test_v2_yaml(self):
799799
rationale: We need this to demonstrate stuff.
800800
repository: https://pagure.io/bar.git
801801
cache: https://example.com/cache
802-
ref: "26ca0c0"
802+
ref: 26ca0c0
803803
baz:
804804
rationale: This one is here to demonstrate other stuff.
805805
xxx:
@@ -1039,7 +1039,7 @@ def test_v1_yaml(self):
10391039
rationale: We need this to demonstrate stuff.
10401040
repository: https://pagure.io/bar.git
10411041
cache: https://example.com/cache
1042-
ref: "26ca0c0"
1042+
ref: 26ca0c0
10431043
baz:
10441044
rationale: This one is here to demonstrate other stuff.
10451045
xxx:

‎modulemd/tests/test-modulemd-modulestream.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,7 @@ module_stream_test_v2_yaml (void)
796796
" rationale: We need this to demonstrate stuff.\n"
797797
" repository: https://pagure.io/bar.git\n"
798798
" cache: https://example.com/cache\n"
799-
" ref: \"26ca0c0\"\n"
799+
" ref: 26ca0c0\n"
800800
" baz:\n"
801801
" rationale: This one is here to demonstrate other stuff.\n"
802802
" xxx:\n"
@@ -2595,7 +2595,7 @@ module_stream_v1_test_parse_dump (void)
25952595
" rationale: We need this to demonstrate stuff.\n"
25962596
" repository: https://pagure.io/bar.git\n"
25972597
" cache: https://example.com/cache\n"
2598-
" ref: \"26ca0c0\"\n"
2598+
" ref: 26ca0c0\n"
25992599
" baz:\n"
26002600
" rationale: This one is here to demonstrate other stuff.\n"
26012601
" xxx:\n"
@@ -2787,7 +2787,7 @@ module_stream_v2_test_parse_dump (void)
27872787
" name: bar-real\n"
27882788
" repository: https://pagure.io/bar.git\n"
27892789
" cache: https://example.com/cache\n"
2790-
" ref: \"26ca0c0\"\n"
2790+
" ref: 26ca0c0\n"
27912791
" baz:\n"
27922792
" rationale: Demonstrate updating the buildroot contents.\n"
27932793
" buildroot: true\n"
@@ -2822,7 +2822,7 @@ module_stream_v2_test_parse_dump (void)
28222822
" name: bar\n"
28232823
" epoch: 0\n"
28242824
" version: \"1.23\"\n"
2825-
" release: \"1.module_deadbeef\"\n"
2825+
" release: 1.module_deadbeef\n"
28262826
" arch: x86_64\n"
28272827
" nevra: bar-0:1.23-1.module_deadbeef.x86_64\n"
28282828
"...\n");

‎modulemd/tests/test-modulemd-packager-v3.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ validate_yaml (ModulemdPackagerV3 *packager)
387387
" name: bar-real\n"
388388
" repository: https://pagure.io/bar.git\n"
389389
" cache: https://example.com/cache\n"
390-
" ref: \"26ca0c0\"\n"
390+
" ref: 26ca0c0\n"
391391
" baz:\n"
392392
" rationale: Demonstrate updating the buildroot contents.\n"
393393
" buildroot: true\n"

‎modulemd/tests/test-modulemd-quoting.c‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
/*
2+
* This file is part of libmodulemd
3+
* Copyright (C) 2023 Red Hat, Inc.
4+
*
5+
* Fedora-License-Identifier: MIT
6+
* SPDX-2.0-License-Identifier: MIT
7+
* SPDX-3.0-License-Identifier: MIT
8+
*
9+
* This program is free software.
10+
* For more information on the license, see COPYING.
11+
* For more information on free software, see <https://www.gnu.org/philosophy/free-sw.en.html>.
12+
*/
13+
14+
#include <glib.h>
15+
#include <locale.h>
16+
#include "modulemd-profile.h"
17+
#include "private/modulemd-yaml.h"
18+
#include "private/modulemd-profile-private.h"
19+
20+
/*#include <glib/gstdio.h>
21+
22+
23+
#include "private/glib-extensions.h"
24+
#include "private/test-utils.h"*/
25+
26+
struct item
27+
{
28+
const char *input;
29+
int quoted; /* true for quoting expected, otherwise unquoted expected */
30+
};
31+
32+
/*
33+
* Test that strings only consisting of a number are quoted to prevent
34+
* consumers from interpretting them as a number and thus mangling the
35+
* string value by normalizing the number.
36+
* Ability to quote numerical strings at each part of a YAML document is
37+
* tested in tests for the particular document type, i.e. not in this file.
38+
* This code uses an RPM package list of a stream profile for the purpose
39+
* of testing. It's the most brief usage of quoting.
40+
*/
41+
static void
42+
test_quoting (gconstpointer data)
43+
{
44+
const struct item *test_case = (const struct item *)data;
45+
g_autoptr (ModulemdProfile) p = NULL;
46+
g_autoptr (GError) error = NULL;
47+
MMD_INIT_YAML_EMITTER (emitter);
48+
MMD_INIT_YAML_STRING (&emitter, yaml_string);
49+
g_autoptr (GString) expected = g_string_new (NULL);
50+
51+
if (test_case->quoted)
52+
g_string_printf (expected,
53+
"---\n"
54+
"\"0\":\n"
55+
" rpms:\n"
56+
" - \"%s\"\n"
57+
"...\n",
58+
test_case->input);
59+
else
60+
g_string_printf (expected,
61+
"---\n"
62+
"\"0\":\n"
63+
" rpms:\n"
64+
" - %s\n"
65+
"...\n",
66+
test_case->input);
67+
68+
p = modulemd_profile_new ("0");
69+
70+
modulemd_profile_add_rpm (p, test_case->input);
71+
72+
g_assert_true (mmd_emitter_start_stream (&emitter, &error));
73+
g_assert_true (mmd_emitter_start_document (&emitter, &error));
74+
g_assert_true (
75+
mmd_emitter_start_mapping (&emitter, YAML_BLOCK_MAPPING_STYLE, &error));
76+
g_assert_true (modulemd_profile_emit_yaml (p, &emitter, &error));
77+
g_assert_true (mmd_emitter_end_mapping (&emitter, &error));
78+
g_assert_true (mmd_emitter_end_document (&emitter, &error));
79+
g_assert_true (mmd_emitter_end_stream (&emitter, &error));
80+
if (g_strcmp0 (yaml_string->str, expected->str))
81+
{
82+
g_test_message (
83+
"Expected=\"%s\"\nGot=\"%s\"", expected->str, yaml_string->str);
84+
g_test_fail ();
85+
}
86+
}
87+
88+
89+
int
90+
main (int argc, char *argv[])
91+
{
92+
/* clang-format off */
93+
struct item test_cases[] = {
94+
{"0", 1}, /* YAML/JSON floats */
95+
{"0.", 1},
96+
{"0.0", 1},
97+
{".0", 1},
98+
{"-1", 1},
99+
{"-1.", 1},
100+
{"-1.0", 1},
101+
{"-.0", 1},
102+
{"+1", 1}, /* Handle "+" for sure */
103+
{"+1.", 1},
104+
{"+1.0", 1},
105+
{"+.0", 1},
106+
{"1.0e1", 1},
107+
{"-1.0e1", 1},
108+
{"+1.0e1", 1},
109+
{"1.0e-1", 1},
110+
{"-1.0e-1", 1},
111+
{"+1.0e-1", 1},
112+
{"1.0e+1", 1},
113+
{"-1.0e+1", 1},
114+
{"+1.0e+1", 1},
115+
{".inf", 1},
116+
{"-.inf", 1},
117+
{"+.inf", 1},
118+
{".nan", 1},
119+
120+
{"0x", 0}, /* Incomplete hexadecimal */
121+
{"0x0", 1}, /* YAML hexadecicmal notation */
122+
{"0xa", 1},
123+
{"0xA", 1},
124+
{"0xg", 0}, /* Invalid hexadecimal */
125+
126+
{"0o", 0}, /* Incomplete octal */
127+
{"0o0", 1}, /* YAML octal notation */
128+
{"0o8", 0}, /* Invalid octal */
129+
130+
{"0a", 0}, /* This does not need quoting. Common in refs. */
131+
{NULL, 0}
132+
};
133+
/* clang-format on */
134+
struct item *test_case;
135+
g_autoptr (GString) testpath = g_string_new (NULL);
136+
137+
setlocale (LC_ALL, "");
138+
g_test_init (&argc, &argv, NULL);
139+
g_test_set_nonfatal_assertions ();
140+
141+
for (test_case = test_cases; test_case->input != NULL; test_case++)
142+
{
143+
g_string_printf (
144+
testpath, "/modulemd/yaml/quoting/%s", test_case->input);
145+
g_test_add_data_func (
146+
testpath->str, (gconstpointer)test_case, test_quoting);
147+
}
148+
149+
return g_test_run ();
150+
}

‎modulemd/tests/test-modulemd-rpmmap.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ test_emit_yaml_valid (void)
221221
"name: baz\n"
222222
"epoch: 2\n"
223223
"version: \"2.18\"\n"
224-
"release: \"3.module_baddad\"\n"
224+
"release: 3.module_baddad\n"
225225
"arch: s390x\n"
226226
"nevra: baz-2:2.18-3.module_baddad.s390x\n"
227227
"...\n";
@@ -279,7 +279,7 @@ test_emit_yaml_quoting (void)
279279
"version: \"2\"\n"
280280
"release: \"3\"\n"
281281
"arch: \"4\"\n"
282-
"nevra: \"0-1:2-3.4\"\n"
282+
"nevra: 0-1:2-3.4\n"
283283
"...\n";
284284

285285
entry = modulemd_rpm_map_entry_new ("0", 1, "2", "3", "4");

‎modulemd/tests/test_data/component-rpm/cr.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ bar:
44
repository: https://pagure.io/bar.git
55
cache: https://example.com/cache
66
buildorder: 100
7-
ref: "26ca0c0"
7+
ref: 26ca0c0
88
buildroot: true
99
srpm-buildroot: true
1010
arches: [i686, x86_64]

‎modulemd/tests/test_data/component-rpm/cr_buildafter.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ bar:
55
cache: https://example.com/cache
66
buildafter:
77
- foo
8-
ref: "26ca0c0"
8+
ref: 26ca0c0
99
buildroot: true
1010
srpm-buildroot: true
1111
arches: [i686, x86_64]

‎modulemd/tests/test_data/good-v2-extra-keys.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ data:
222222
rationale: We need this to demonstrate stuff.
223223
repository: https://pagure.io/bar.git
224224
cache: https://example.com/cache
225-
ref: "26ca0c0"
225+
ref: 26ca0c0
226226
baz:
227227
rationale: This one is here to demonstrate other stuff.
228228
xxx:

0 commit comments

Comments
 (0)