github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-23 08:04:03 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FIX: custom SQL with a trailing comment might break BadgeGranter SQL (#9285)

Browse Source 
For example given a custom badge with SQL:
```
SELECT 1
-- I am a comment
```

You end up with

```
FROM (SELECT 1
-- I am a comment) q
```

This fix adds newlines so you end up with the now-valid:

```
FROM (
  SELECT 1
-- I am a comment
) q
```
This commit is contained in:
Michael Brown 2020-03-27 14:16:14 -04:00 committed by GitHub
parent 702879cbda
commit 9026c55fe4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 36 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
app/services/badge_granter.rb
View File

@ -222,13 +222,21 @@ class BadgeGranter
BadgeGranter.contract_checks!(sql, opts) BadgeGranter.contract_checks!(sql, opts)
# hack to allow for params, otherwise sanitizer will trigger sprintf # hack to allow for params, otherwise sanitizer will trigger sprintf
count_sql = "SELECT COUNT(*) count FROM (#{sql}) q WHERE :backfill = :backfill" count_sql = <<~SQL
SELECT COUNT(*) count
FROM (
#{sql}
) q
WHERE :backfill = :backfill
SQL
grant_count = DB.query_single(count_sql, params).first.to_i grant_count = DB.query_single(count_sql, params).first.to_i
grants_sql = if opts[:target_posts] grants_sql = if opts[:target_posts]
<<~SQL <<~SQL
SELECT u.id, u.username, q.post_id, t.title, q.granted_at SELECT u.id, u.username, q.post_id, t.title, q.granted_at
FROM (#{sql}) q FROM (
#{sql}
) q
JOIN users u on u.id = q.user_id JOIN users u on u.id = q.user_id
LEFT JOIN badge_posts p on p.id = q.post_id LEFT JOIN badge_posts p on p.id = q.post_id
LEFT JOIN topics t on t.id = p.topic_id LEFT JOIN topics t on t.id = p.topic_id
@ -238,7 +246,9 @@ class BadgeGranter
else else
<<~SQL <<~SQL
SELECT u.id, u.username, q.granted_at SELECT u.id, u.username, q.granted_at
FROM (#{sql}) q FROM (
#{sql}
) q
JOIN users u on u.id = q.user_id JOIN users u on u.id = q.user_id
WHERE :backfill = :backfill WHERE :backfill = :backfill
LIMIT 10 LIMIT 10
@ -293,13 +303,15 @@ class BadgeGranter
sql = <<~SQL sql = <<~SQL
DELETE FROM user_badges DELETE FROM user_badges
WHERE id IN ( WHERE id IN (
SELECT ub.id SELECT ub.id
FROM user_badges ub FROM user_badges ub
LEFT JOIN (#{badge.query}) q ON q.user_id = ub.user_id LEFT JOIN (
#{post_clause} #{badge.query}
WHERE ub.badge_id = :id AND q.user_id IS NULL ) q ON q.user_id = ub.user_id
) #{post_clause}
WHERE ub.badge_id = :id AND q.user_id IS NULL
)
SQL SQL
DB.exec( DB.exec(
@ -315,7 +327,9 @@ class BadgeGranter
WITH w as ( WITH w as (
INSERT INTO user_badges(badge_id, user_id, granted_at, granted_by_id, post_id) INSERT INTO user_badges(badge_id, user_id, granted_at, granted_by_id, post_id)
SELECT :id, q.user_id, q.granted_at, -1, #{post_id_field} SELECT :id, q.user_id, q.granted_at, -1, #{post_id_field}
FROM (#{badge.query}) q FROM (
#{badge.query}
) q
LEFT JOIN user_badges ub ON ub.badge_id = :id AND ub.user_id = q.user_id LEFT JOIN user_badges ub ON ub.badge_id = :id AND ub.user_id = q.user_id
#{post_clause} #{post_clause}
/*where*/ /*where*/

11
spec/services/badge_granter_spec.rb
View File

@ -47,6 +47,11 @@ describe BadgeGranter do
expect(result[:grant_count]).to eq(1) expect(result[:grant_count]).to eq(1)
expect(result[:query_plan]).to be_present expect(result[:query_plan]).to be_present
end end
it 'with badges containing trailing comments do not break generated SQL' do
query = Badge.find(1).query + "\n-- a comment"
expect(BadgeGranter.preview(query)[:errors]).to be_nil
end
end end
describe 'backfill' do describe 'backfill' do
@ -114,6 +119,12 @@ describe BadgeGranter do
expect(notification_badge_name).not_to eq(name_english) expect(notification_badge_name).not_to eq(name_english)
end end
it 'with badges containing trailing comments do not break generated SQL' do
badge = Fabricate(:badge)
badge.query = Badge.find(1).query + "\n-- a comment"
expect { BadgeGranter.backfill(badge) }.not_to raise_error
end
end end
describe 'grant' do describe 'grant' do