Skip to content

Commit 4459a20

Browse files
protobuf-github-botcopybara-github
protobuf-github-bot
authored and
copybara-github
committed
Support more chars in type URLs in the Python text-format parser.
Change the Python text-format parser to allow for more characters and formats in the type URL prefixes of expanded Any protos. This follows a recent change to the text-format spec which we are now closely following. Refs: - [1] https://protobuf.dev/reference/protobuf/textformat-spec/#characters - [2] https://protobuf.dev/reference/protobuf/textformat-spec/#field- PiperOrigin-RevId: 844614988
1 parent ee9f0bc commit 4459a20

File tree

2 files changed

+331
-59
lines changed

2 files changed

+331
-59
lines changed

‎python/google/protobuf/internal/text_format_test.py‎

Lines changed: 222 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1741,26 +1741,26 @@ def testParseAllowedUnknownExtension(self):
17411741

17421742
# Fail if invalid Any message type url inside unknown extensions.
17431743
message = any_test_pb2.TestAny()
1744-
text = ('any_value {\n'
1745-
' [type.googleapis.com.invalid/google.protobuf.internal.TestAny] {\n'
1746-
' [unknown_extension] {\n'
1747-
' str: "string"\n'
1748-
' any_value {\n'
1749-
' [type.googleapis.com/proto2_unittest.OneString] {\n'
1750-
' data: "string"\n'
1751-
' }\n'
1752-
' }\n'
1753-
' }\n'
1754-
' }\n'
1755-
'}\n'
1756-
'int32_value: 123')
1757-
self.assertRaisesRegex(
1744+
text = (
1745+
'any_value {\n'
1746+
' [invalid@prefix/google.protobuf.internal.TestAny] {\n'
1747+
' [unknown_extension] {\n'
1748+
' str: "string"\n'
1749+
' any_value {\n'
1750+
' [type.googleapis.com/proto2_unittest.OneString] {\n'
1751+
' data: "string"\n'
1752+
' }\n'
1753+
' }\n'
1754+
' }\n'
1755+
' }\n'
1756+
'}\n'
1757+
'int32_value: 123'
1758+
)
1759+
with self.assertRaisesRegex(
17581760
text_format.ParseError,
1759-
'[type.googleapis.com.invalid/google.protobuf.internal.TestAny]',
1760-
text_format.Parse,
1761-
text,
1762-
message,
1763-
allow_unknown_extension=True)
1761+
'[invalid@prefix/google.protobuf.internal.TestAny]',
1762+
):
1763+
text_format.Parse(text, message, allow_unknown_extension=True)
17641764

17651765
def testParseBadIdentifier(self):
17661766
message = unittest_pb2.TestAllTypes()
@@ -1885,21 +1885,23 @@ def testParseMap(self):
18851885
self.assertEqual(5, message.map_int32_foreign_message[111].c)
18861886

18871887

1888-
class Proto3Tests(unittest.TestCase):
1888+
class Proto3Tests(parameterized.TestCase):
18891889

18901890
def testPrintMessageExpandAny(self):
18911891
packed_message = unittest_pb2.OneString()
18921892
packed_message.data = 'string'
18931893
message = any_test_pb2.TestAny()
18941894
message.any_value.Pack(packed_message)
18951895
self.assertEqual(
1896-
text_format.MessageToString(message,
1897-
descriptor_pool=descriptor_pool.Default()),
1896+
text_format.MessageToString(
1897+
message, descriptor_pool=descriptor_pool.Default()
1898+
),
18981899
'any_value {\n'
18991900
' [type.googleapis.com/proto2_unittest.OneString] {\n'
19001901
' data: "string"\n'
19011902
' }\n'
1902-
'}\n')
1903+
'}\n',
1904+
)
19031905

19041906
def testPrintStructInAny(self):
19051907
packed_message = struct_pb2.Struct()
@@ -2084,17 +2086,205 @@ def testMergeExpandedAnyPointyBrackets(self):
20842086
message.any_value.Unpack(packed_message)
20852087
self.assertEqual('string', packed_message.data)
20862088

2087-
def testMergeAlternativeUrl(self):
2089+
@parameterized.parameters(
2090+
{
2091+
'any_name': '[domain.com/proto2_unittest.OneString]',
2092+
'type_url': 'domain.com/proto2_unittest.OneString',
2093+
},
2094+
# Multiple slashes in prefix
2095+
{
2096+
'any_name': '[domain.com/path/proto2_unittest.OneString]',
2097+
'type_url': 'domain.com/path/proto2_unittest.OneString',
2098+
},
2099+
{
2100+
'any_name': '[domain.com///path//proto2_unittest.OneString]',
2101+
'type_url': 'domain.com///path//proto2_unittest.OneString',
2102+
},
2103+
# Special characters in prefix
2104+
{
2105+
'any_name': '[domain.com/-.~_!$&()*+,;=/proto2_unittest.OneString]',
2106+
'type_url': 'domain.com/-.~_!$&()*+,;=/proto2_unittest.OneString',
2107+
},
2108+
# Percent escapes in prefix
2109+
{
2110+
'any_name': (
2111+
'[percent.escapes/%0a%1B%2c%3D%4e%F5%A6%b7%C8%f9/proto2_unittest.OneString]'
2112+
),
2113+
'type_url': (
2114+
'percent.escapes/%0a%1B%2c%3D%4e%F5%A6%b7%C8%f9/proto2_unittest.OneString'
2115+
),
2116+
},
2117+
# Whitespace and comments (should be ignored between [])
2118+
{
2119+
'any_name': '[ domain . com / proto2_ unittest. One String ]',
2120+
'type_url': 'domain.com/proto2_unittest.OneString',
2121+
},
2122+
{
2123+
'any_name': (
2124+
'[ \t\n\r\f\v domain.com/pr \t\n\r\f\v oto2_unittest.OneString'
2125+
' \t\n\r\f\v ]'
2126+
),
2127+
'type_url': 'domain.com/proto2_unittest.OneString',
2128+
},
2129+
{
2130+
'any_name': (
2131+
'[ # comment\n domain.com/pr # comment\n oto2_unittest.One String'
2132+
' # comment\n ]'
2133+
),
2134+
'type_url': 'domain.com/proto2_unittest.OneString',
2135+
},
2136+
)
2137+
def testMergeExpandedAnyTypeUrls(self, *, any_name, type_url):
20882138
message = any_test_pb2.TestAny()
2089-
text = ('any_value {\n'
2090-
' [type.otherapi.com/proto2_unittest.OneString] {\n'
2091-
' data: "string"\n'
2092-
' }\n'
2093-
'}\n')
2139+
text = f'any_value {{\n {any_name} {{\n data: "string"\n }}\n }}'
2140+
20942141
text_format.Merge(text, message)
2095-
packed_message = unittest_pb2.OneString()
2096-
self.assertEqual('type.otherapi.com/proto2_unittest.OneString',
2097-
message.any_value.type_url)
2142+
self.assertEqual(type_url, message.any_value.type_url)
2143+
2144+
@parameterized.parameters(
2145+
# General error cases
2146+
{
2147+
'any_name': '[',
2148+
'error_msg': '2:4 : \' [ {\': Expected "]"',
2149+
},
2150+
{
2151+
'any_name': '[]',
2152+
'error_msg': '2:5 : \' [] {\': Type URL does not contain "/"',
2153+
},
2154+
{
2155+
'any_name': '[.type]',
2156+
'error_msg': '2:10 : \' [.type] {\': Type URL does not contain "/"',
2157+
},
2158+
# Prefix error cases
2159+
{
2160+
'any_name': '[/]',
2161+
'error_msg': "2:6 : ' [/] {': Type URL prefix is empty.",
2162+
},
2163+
{
2164+
'any_name': '[/proto2_unittest.OneString]',
2165+
'error_msg': (
2166+
"2:31 : ' [/proto2_unittest.OneString] {': "
2167+
'Type URL prefix is empty'
2168+
),
2169+
},
2170+
{
2171+
'any_name': '[/domain.com/proto2_unittest.OneString]',
2172+
'error_msg': (
2173+
"2:42 : ' [/domain.com/proto2_unittest.OneString] {': "
2174+
'Type URL prefix starts with "/"'
2175+
),
2176+
},
2177+
# Special characters in prefix
2178+
{
2179+
'any_name': '[domain.com/?/proto2_unittest.OneString]',
2180+
'error_msg': (
2181+
"2:14 : ' [domain.com/?/proto2_unittest.OneString] {':"
2182+
' Expected "]"'
2183+
),
2184+
},
2185+
{
2186+
'any_name': '[domain.com/:/proto2_unittest.OneString]',
2187+
'error_msg': (
2188+
"2:14 : ' [domain.com/:/proto2_unittest.OneString] {':"
2189+
' Expected "]"'
2190+
),
2191+
},
2192+
{
2193+
'any_name': '[domain.com/@/proto2_unittest.OneString]',
2194+
'error_msg': (
2195+
"2:14 : ' [domain.com/@/proto2_unittest.OneString] {': "
2196+
'Expected "]".'
2197+
),
2198+
},
2199+
{
2200+
'any_name': '[domain.com/@/proto2_unittest.OneString]',
2201+
'error_msg': (
2202+
"2:14 : ' [domain.com/@/proto2_unittest.OneString] {':"
2203+
' Expected "]"'
2204+
),
2205+
},
2206+
# Percent escapes in prefix
2207+
{
2208+
'any_name': '[percent.escapes/%/proto2_unittest.OneString]',
2209+
'error_msg': (
2210+
"2:48 : ' [percent.escapes/%/proto2_unittest.OneString] {':"
2211+
' Invalid percent escape, got "%".'
2212+
),
2213+
},
2214+
{
2215+
'any_name': '[percent.escapes/%G/proto2_unittest.OneString]',
2216+
'error_msg': (
2217+
"2:49 : ' [percent.escapes/%G/proto2_unittest.OneString] {':"
2218+
' Invalid percent escape, got "%G".'
2219+
),
2220+
},
2221+
{
2222+
'any_name': '[percent.escapes/%aG/proto2_unittest.OneString]',
2223+
'error_msg': (
2224+
"2:50 : ' [percent.escapes/%aG/proto2_unittest.OneString] {':"
2225+
' Invalid percent escape, got "%aG".'
2226+
),
2227+
},
2228+
# Invalid type names
2229+
{
2230+
'any_name': '[domain.com/]',
2231+
'error_msg': (
2232+
'2:16 : \' [domain.com/] {\': Expected type name, got "".'
2233+
),
2234+
},
2235+
{
2236+
'any_name': '[domain.com/.]',
2237+
'error_msg': (
2238+
'2:17 : \' [domain.com/.] {\': Expected type name, got ".".'
2239+
),
2240+
},
2241+
{
2242+
'any_name': '[domain.com/.OneString]',
2243+
'error_msg': (
2244+
"2:26 : ' [domain.com/.OneString] {': "
2245+
'Expected type name, got ".OneString".'
2246+
),
2247+
},
2248+
{
2249+
'any_name': '[domain.com/proto2_unittest.]',
2250+
'error_msg': (
2251+
"2:32 : ' [domain.com/proto2_unittest.] {': "
2252+
'Expected type name, got "proto2_unittest.".'
2253+
),
2254+
},
2255+
{
2256+
'any_name': '[domain.com/5type]',
2257+
'error_msg': (
2258+
"2:21 : ' [domain.com/5type] {': "
2259+
'Expected type name, got "5type".'
2260+
),
2261+
},
2262+
{
2263+
'any_name': '[domain.com/!]',
2264+
'error_msg': (
2265+
'2:17 : \' [domain.com/!] {\': Expected type name, got "!".'
2266+
),
2267+
},
2268+
{
2269+
'any_name': '[domain.com/my_?_type]',
2270+
'error_msg': '2:17 : \' [domain.com/my_?_type] {\': Expected "]".',
2271+
},
2272+
{
2273+
'any_name': '[domain.com/my.:type]',
2274+
'error_msg': '2:17 : \' [domain.com/my.:type] {\': Expected "]".',
2275+
},
2276+
{
2277+
'any_name': '[domain.com/my.type@]',
2278+
'error_msg': '2:21 : \' [domain.com/my.type@] {\': Expected "]".',
2279+
},
2280+
)
2281+
def testMergeFailsOnInvalidExpandedAnyTypeUrls(self, *, any_name, error_msg):
2282+
message = any_test_pb2.TestAny()
2283+
text = 'any_value {\n %s {\n data: "string"\n }\n }' % any_name
2284+
2285+
with self.assertRaises(text_format.ParseError) as e:
2286+
text_format.Merge(text, message)
2287+
self.assertIn(error_msg, str(e.exception))
20982288

20992289
def testMergeExpandedAnyDescriptorPoolMissingType(self):
21002290
message = any_test_pb2.TestAny()

0 commit comments

Comments
 (0)