Skip to content

Commit 75da739

Browse files
soedirgosoedirgo
committed
fix: identifier double escaping
`::regclass` already escapes identifier names, no need to `%I` it
1 parent 34dd105 commit 75da739

File tree

4 files changed

+372
-8
lines changed

4 files changed

+372
-8
lines changed

‎src/lib/PostgresMetaColumnPrivileges.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ where a.attrelid = ${literal(relationId)}
6969
and a.attnum = ${literal(columnNumber)}
7070
into col;
7171
execute format(
72-
'grant ${privilege_type} (%I) on %I to ${
72+
'grant ${privilege_type} (%I) on %s to ${
7373
grantee.toLowerCase() === 'public' ? 'public' : ident(grantee)
7474
} ${is_grantable ? 'with grant option' : ''}',
7575
col.attname,
@@ -113,7 +113,7 @@ where a.attrelid = ${literal(relationId)}
113113
and a.attnum = ${literal(columnNumber)}
114114
into col;
115115
execute format(
116-
'revoke ${privilege_type} (%I) on %I from ${
116+
'revoke ${privilege_type} (%I) on %s from ${
117117
grantee.toLowerCase() === 'public' ? 'public' : ident(grantee)
118118
}',
119119
col.attname,

‎src/lib/PostgresMetaTablePrivileges.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ begin
115115
${grants
116116
.map(
117117
({ privilege_type, relation_id, grantee, is_grantable }) =>
118-
`execute format('grant ${privilege_type} on table %I to ${
118+
`execute format('grant ${privilege_type} on table %s to ${
119119
grantee.toLowerCase() === 'public' ? 'public' : ident(grantee)
120120
} ${is_grantable ? 'with grant option' : ''}', ${relation_id}::regclass);`
121121
)
@@ -147,7 +147,7 @@ begin
147147
${revokes
148148
.map(
149149
(revoke) =>
150-
`execute format('revoke ${revoke.privilege_type} on table %I from ${revoke.grantee}', ${revoke.relation_id}::regclass);`
150+
`execute format('revoke ${revoke.privilege_type} on table %s from ${revoke.grantee}', ${revoke.relation_id}::regclass);`
151151
)
152152
.join('\n')}
153153
end $$;

‎test/server/column-privileges.ts

Lines changed: 157 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ test('revoke & grant column privileges', async () => {
8181
],
8282
})
8383
let privs = res.json<PostgresColumnPrivileges[]>()
84-
expect(privs.length === 1)
84+
expect(privs.length).toBe(1)
8585
expect(privs[0]).toMatchInlineSnapshot(
8686
{ column_id: expect.stringMatching(/^\d+\.\d+$/) },
8787
`
@@ -156,7 +156,7 @@ test('revoke & grant column privileges', async () => {
156156
],
157157
})
158158
privs = res.json<PostgresColumnPrivileges[]>()
159-
expect(privs.length === 1)
159+
expect(privs.length).toBe(1)
160160
expect(privs[0]).toMatchInlineSnapshot(
161161
{ column_id: expect.stringMatching(/^\d+\.\d+$/) },
162162
`
@@ -206,3 +206,158 @@ test('revoke & grant column privileges', async () => {
206206
throw new Error(res.payload)
207207
}
208208
})
209+
210+
test('revoke & grant column privileges w/ quoted column name', async () => {
211+
let res = await app.inject({
212+
method: 'POST',
213+
path: '/query',
214+
payload: {
215+
query: `create role r; create table "t 1"("c 1" int8);`,
216+
},
217+
})
218+
if (res.json().error) {
219+
throw new Error(res.payload)
220+
}
221+
222+
res = await app.inject({ method: 'GET', path: '/column-privileges' })
223+
const { column_id } = res
224+
.json<PostgresColumnPrivileges[]>()
225+
.find(({ relation_name, column_name }) => relation_name === 't 1' && column_name === 'c 1')!
226+
227+
res = await app.inject({
228+
method: 'POST',
229+
path: '/column-privileges',
230+
payload: [
231+
{
232+
column_id,
233+
grantee: 'r',
234+
privilege_type: 'ALL',
235+
},
236+
],
237+
})
238+
let privs = res.json<PostgresColumnPrivileges[]>()
239+
expect(privs.length).toBe(1)
240+
expect(privs[0]).toMatchInlineSnapshot(
241+
{ column_id: expect.stringMatching(/^\d+\.\d+$/) },
242+
`
243+
{
244+
"column_id": StringMatching /\\^\\\\d\\+\\\\\\.\\\\d\\+\\$/,
245+
"column_name": "c 1",
246+
"privileges": [
247+
{
248+
"grantee": "r",
249+
"grantor": "postgres",
250+
"is_grantable": false,
251+
"privilege_type": "UPDATE",
252+
},
253+
{
254+
"grantee": "r",
255+
"grantor": "postgres",
256+
"is_grantable": false,
257+
"privilege_type": "SELECT",
258+
},
259+
{
260+
"grantee": "r",
261+
"grantor": "postgres",
262+
"is_grantable": false,
263+
"privilege_type": "REFERENCES",
264+
},
265+
{
266+
"grantee": "r",
267+
"grantor": "postgres",
268+
"is_grantable": false,
269+
"privilege_type": "INSERT",
270+
},
271+
{
272+
"grantee": "postgres",
273+
"grantor": "postgres",
274+
"is_grantable": false,
275+
"privilege_type": "UPDATE",
276+
},
277+
{
278+
"grantee": "postgres",
279+
"grantor": "postgres",
280+
"is_grantable": false,
281+
"privilege_type": "SELECT",
282+
},
283+
{
284+
"grantee": "postgres",
285+
"grantor": "postgres",
286+
"is_grantable": false,
287+
"privilege_type": "REFERENCES",
288+
},
289+
{
290+
"grantee": "postgres",
291+
"grantor": "postgres",
292+
"is_grantable": false,
293+
"privilege_type": "INSERT",
294+
},
295+
],
296+
"relation_name": "t 1",
297+
"relation_schema": "public",
298+
}
299+
`
300+
)
301+
302+
res = await app.inject({
303+
method: 'DELETE',
304+
path: '/column-privileges',
305+
payload: [
306+
{
307+
column_id,
308+
grantee: 'r',
309+
privilege_type: 'ALL',
310+
},
311+
],
312+
})
313+
privs = res.json<PostgresColumnPrivileges[]>()
314+
expect(privs.length).toBe(1)
315+
expect(privs[0]).toMatchInlineSnapshot(
316+
{ column_id: expect.stringMatching(/^\d+\.\d+$/) },
317+
`
318+
{
319+
"column_id": StringMatching /\\^\\\\d\\+\\\\\\.\\\\d\\+\\$/,
320+
"column_name": "c 1",
321+
"privileges": [
322+
{
323+
"grantee": "postgres",
324+
"grantor": "postgres",
325+
"is_grantable": false,
326+
"privilege_type": "UPDATE",
327+
},
328+
{
329+
"grantee": "postgres",
330+
"grantor": "postgres",
331+
"is_grantable": false,
332+
"privilege_type": "SELECT",
333+
},
334+
{
335+
"grantee": "postgres",
336+
"grantor": "postgres",
337+
"is_grantable": false,
338+
"privilege_type": "REFERENCES",
339+
},
340+
{
341+
"grantee": "postgres",
342+
"grantor": "postgres",
343+
"is_grantable": false,
344+
"privilege_type": "INSERT",
345+
},
346+
],
347+
"relation_name": "t 1",
348+
"relation_schema": "public",
349+
}
350+
`
351+
)
352+
353+
res = await app.inject({
354+
method: 'POST',
355+
path: '/query',
356+
payload: {
357+
query: `drop role r; drop table "t 1";`,
358+
},
359+
})
360+
if (res.json().error) {
361+
throw new Error(res.payload)
362+
}
363+
})

0 commit comments

Comments
 (0)