br: Solve SQL Injection Risk - Format String #49666

lyzx2001 · 2023-12-21T07:30:07Z

What problem does this PR solve?

Issue Number: ref #30699

Problem Summary:

What changed and how does it work?

Solve SQL Injection Risk - Format String.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

tiprow · 2023-12-21T07:30:22Z

Hi @lyzx2001. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

br/pkg/lightning/checkpoints/checkpoints.go

codecov · 2023-12-21T07:50:25Z

Codecov Report

Merging #49666 (c08919e) into master (eff1156) will increase coverage by 11.4831%.
Report is 103 commits behind head on master.
The diff coverage is 87.5000%.

Additional details and impacted files

@@                Coverage Diff                @@
##             master     #49666         +/-   ##
=================================================
+ Coverage   70.9388%   82.4220%   +11.4831%     
=================================================
  Files          1368       2444       +1076     
  Lines        396702     675516     +278814     
=================================================
+ Hits         281416     556774     +275358     
- Misses        95593      97666       +2073     
- Partials      19693      21076       +1383

Flag	Coverage Δ
integration	`50.5760% <79.1666%> (?)`
unit	`79.2724% <87.5000%> (+8.3335%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`73.6130% <ø> (+19.6466%)`	⬆️
parser	`∅ <ø> (∅)`
br	`75.2739% <87.5000%> (+22.3706%)`	⬆️

lyzx2001 · 2023-12-21T09:50:48Z

/test pull-lightning-integration-test

tiprow · 2023-12-21T09:51:11Z

@lyzx2001: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test pull-lightning-integration-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lyzx2001 · 2023-12-21T09:52:16Z

/cc @lance6716

lyzx2001 · 2023-12-21T10:00:43Z

/test pull-lightning-integration-test

tiprow · 2023-12-21T10:01:06Z

@lyzx2001: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test pull-lightning-integration-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lyzx2001 · 2023-12-21T10:43:49Z

/retest

tiprow · 2023-12-21T10:44:14Z

@lyzx2001: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lyzx2001 · 2023-12-21T11:29:37Z

/retest

tiprow · 2023-12-21T11:29:59Z

@lyzx2001: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lance6716

also need to update other unit tests I didn't mention

br/pkg/lightning/checkpoints/checkpoints_sql_test.go

merge master

lyzx2001 · 2023-12-28T15:29:54Z

/test pull-lightning-integration-test

tiprow · 2023-12-28T15:30:17Z

@lyzx2001: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test pull-lightning-integration-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lyzx2001 · 2024-01-04T04:43:47Z

/test pull-lightning-integration-test

tiprow · 2024-01-04T04:44:12Z

@lyzx2001: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test pull-lightning-integration-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lyzx2001 · 2024-01-04T06:34:06Z

/retest

tiprow · 2024-01-04T06:34:28Z

@lyzx2001: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lyzx2001 · 2024-01-04T07:01:59Z

/cc @lance6716

lyzx2001 · 2024-01-04T07:18:35Z

/cc @GMHDBJD

GMHDBJD

LGTM

ti-chi-bot · 2024-01-05T02:03:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: GMHDBJD, lance6716

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~br/pkg/lightning/OWNERS~~ [GMHDBJD,lance6716]
~~dumpling/OWNERS~~ [GMHDBJD]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-01-05T02:03:59Z

[LGTM Timeline notifier]

Timeline:

2024-01-04 07:13:56.745155919 +0000 UTC m=+2327527.782382846: ☑️ agreed by lance6716.
2024-01-05 02:03:58.632375566 +0000 UTC m=+2395329.669602493: ☑️ agreed by GMHDBJD.

tiprow · 2024-01-05T02:05:25Z

@lyzx2001: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
tiprow_fast_test	`c08919e`	link	true	`/test tiprow_fast_test`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

okJiang · 2024-01-05T02:59:22Z

ci failed 🤔 @lyzx2001

$ PWD=/home/prow/go/src/github.com/pingcap/tidb  git fetch https://x-access-token:CENSORED@github.com/pingcap/tidb.git c08919e839bde90f53efde1fe369ee718096c0df (runtime: 9.504992595s)
From https://github.com/pingcap/tidb
 * branch                c08919e839bde90f53efde1fe369ee718096c0df -> FETCH_HEAD
$ PWD=/home/prow/go/src/github.com/pingcap/tidb GIT_AUTHOR_DATE=1704394114 GIT_COMMITTER_DATE=1704394114 git merge --no-ff c08919e839bde90f53efde1fe369ee718096c0df (runtime: 11.720358ms)
fatal: refusing to merge unrelated histories

lance6716 · 2024-01-05T03:01:47Z

ci failed 🤔 @lyzx2001

$ PWD=/home/prow/go/src/github.com/pingcap/tidb  git fetch https://x-access-token:CENSORED@github.com/pingcap/tidb.git c08919e839bde90f53efde1fe369ee718096c0df (runtime: 9.504992595s)
From https://github.com/pingcap/tidb
 * branch                c08919e839bde90f53efde1fe369ee718096c0df -> FETCH_HEAD
$ PWD=/home/prow/go/src/github.com/pingcap/tidb GIT_AUTHOR_DATE=1704394114 GIT_COMMITTER_DATE=1704394114 git merge --no-ff c08919e839bde90f53efde1fe369ee718096c0df (runtime: 11.720358ms)
fatal: refusing to merge unrelated histories

Don't worry, tiprow_fast_test is our experimental CI, it will not block merging

ref pingcap#30699

Solve SQL Injection Risk - Format String

3dde39a

ti-chi-bot bot added do-not-merge/invalid-title release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 21, 2023

lyzx2001 changed the title ~~Solve SQL Injection Risk - Format String~~ br: Solve SQL Injection Risk - Format String Dec 21, 2023

ti-chi-bot bot removed the do-not-merge/invalid-title label Dec 21, 2023

Merge branch 'master' into format_string

3a79586

lance6716 reviewed Dec 21, 2023

View reviewed changes

br/pkg/lightning/checkpoints/checkpoints.go Outdated Show resolved Hide resolved

lyzx2001 added 2 commits December 21, 2023 17:22

update checkpoints

1ffceeb

update

ce8f6e4

ti-chi-bot bot added the component/dumpling This is related to Dumpling of TiDB. label Dec 21, 2023

ti-chi-bot bot requested a review from lance6716 December 21, 2023 09:52

update

3890aa1

ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 21, 2023

lance6716 reviewed Dec 21, 2023

View reviewed changes

br/pkg/lightning/checkpoints/checkpoints_sql_test.go Show resolved Hide resolved

br/pkg/lightning/checkpoints/checkpoints_sql_test.go Show resolved Hide resolved

lyzx2001 added 3 commits December 25, 2023 14:21

update

abe2a9f

Merge branch 'master' into format_string

4518aaf

merge master

update

66a8ab6

update

61e7b27

ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 28, 2023

fix bug

c08919e

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 4, 2024

ti-chi-bot bot requested a review from lance6716 January 4, 2024 07:02

lance6716 approved these changes Jan 4, 2024

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jan 4, 2024

ti-chi-bot bot requested a review from GMHDBJD January 4, 2024 07:18

GMHDBJD approved these changes Jan 5, 2024

View reviewed changes

ti-chi-bot bot added approved lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 5, 2024

ti-chi-bot bot merged commit 54d8a14 into pingcap:master Jan 5, 2024
35 of 40 checks passed

AilinKid pushed a commit to AilinKid/tidb that referenced this pull request Jan 17, 2024

br: Solve SQL Injection Risk - Format String (pingcap#49666)

3d01baf

ref pingcap#30699

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

br: Solve SQL Injection Risk - Format String #49666

br: Solve SQL Injection Risk - Format String #49666

lyzx2001 commented Dec 21, 2023

tiprow bot commented Dec 21, 2023

codecov bot commented Dec 21, 2023 •

edited

Loading

lyzx2001 commented Dec 21, 2023

tiprow bot commented Dec 21, 2023

lyzx2001 commented Dec 21, 2023

lyzx2001 commented Dec 21, 2023

tiprow bot commented Dec 21, 2023

lyzx2001 commented Dec 21, 2023

tiprow bot commented Dec 21, 2023

lyzx2001 commented Dec 21, 2023

tiprow bot commented Dec 21, 2023

lance6716 left a comment

lyzx2001 commented Dec 28, 2023

tiprow bot commented Dec 28, 2023

lyzx2001 commented Jan 4, 2024

tiprow bot commented Jan 4, 2024

lyzx2001 commented Jan 4, 2024

tiprow bot commented Jan 4, 2024

lyzx2001 commented Jan 4, 2024

lyzx2001 commented Jan 4, 2024

GMHDBJD left a comment

ti-chi-bot bot commented Jan 5, 2024

ti-chi-bot bot commented Jan 5, 2024

tiprow bot commented Jan 5, 2024

okJiang commented Jan 5, 2024 •

edited

Loading

lance6716 commented Jan 5, 2024

br: Solve SQL Injection Risk - Format String #49666

br: Solve SQL Injection Risk - Format String #49666

Conversation

lyzx2001 commented Dec 21, 2023

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Dec 21, 2023

codecov bot commented Dec 21, 2023 • edited Loading

Codecov Report

lyzx2001 commented Dec 21, 2023

tiprow bot commented Dec 21, 2023

lyzx2001 commented Dec 21, 2023

lyzx2001 commented Dec 21, 2023

tiprow bot commented Dec 21, 2023

lyzx2001 commented Dec 21, 2023

tiprow bot commented Dec 21, 2023

lyzx2001 commented Dec 21, 2023

tiprow bot commented Dec 21, 2023

lance6716 left a comment

Choose a reason for hiding this comment

lyzx2001 commented Dec 28, 2023

tiprow bot commented Dec 28, 2023

lyzx2001 commented Jan 4, 2024

tiprow bot commented Jan 4, 2024

lyzx2001 commented Jan 4, 2024

tiprow bot commented Jan 4, 2024

lyzx2001 commented Jan 4, 2024

lyzx2001 commented Jan 4, 2024

GMHDBJD left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Jan 5, 2024

ti-chi-bot bot commented Jan 5, 2024

[LGTM Timeline notifier]

tiprow bot commented Jan 5, 2024

okJiang commented Jan 5, 2024 • edited Loading

lance6716 commented Jan 5, 2024

codecov bot commented Dec 21, 2023 •

edited

Loading

okJiang commented Jan 5, 2024 •

edited

Loading