Skip to content

Conversation

dpgaspar
Copy link
Member

SUMMARY

Bumps sqlparse to 0.5.0 to address a potencial vulnerability.

Followup from: #28042

Don't think we will have breaking changes here, I think that our upper bound was a cautions bound following semantic versioning for 0.X.X versions.

Changelog: https://github.com/andialbrecht/sqlparse/blob/master/CHANGELOG

Notable Changes

* Drop support for Python 3.5, 3.6, and 3.7.
* Python 3.12 is now supported (pr725, by hugovk).
* IMPORTANT: Fixes a potential denial of service attack (DOS) due to recursion
  error for deeply nested statements. Instead of recursion error a generic
  SQLParseError is raised. See the security advisory for details:
  https://github.com/andialbrecht/sqlparse/security/advisories/GHSA-2m57-hf25-phgg
  The vulnerability was discovered by @uriyay-jfrog. Thanks for reporting!

Enhancements:

* Splitting statements now allows to remove the semicolon at the end.
  Some database backends love statements without semicolon (issue742).
* Support TypedLiterals in get_parameters (pr649, by Khrol).
* Improve splitting of Transact SQL when using GO keyword (issue762).
* Support for some JSON operators (issue682).
* Improve formatting of statements containing JSON operators (issue542).
* Support for BigQuery and Snowflake keywords (pr699, by griffatrasgo).
* Support parsing of OVER clause (issue701, pr768 by r33s3n6).

Bug Fixes

* Ignore dunder attributes when creating Tokens (issue672).
* Allow operators to precede dollar-quoted strings (issue763).
* Fix parsing of nested order clauses (issue745, pr746 by john-bodley).
* Thread-safe initialization of Lexer class (issue730).
* Classify TRUNCATE as DDL and GRANT/REVOKE as DCL keywords (based on pr719
  by josuc1, thanks for bringing this up!).
* Fix parsing of PRIMARY KEY (issue740).

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@john-bodley
Copy link
Member

@dpgaspar and @betodealmeida is the plan sitll—per [SIP-117] Improve SQL parsing —to remove sqlparse completely?

@john-bodley john-bodley added the review:checkpoint Last PR reviewed during the daily review standup label Apr 19, 2024
@dpgaspar dpgaspar requested a review from mistercrunch May 14, 2024 07:50
pyproject.toml Outdated
@@ -89,7 +89,7 @@ dependencies = [
"sqlalchemy>=1.4, <2",
"sqlalchemy-utils>=0.38.3, <0.39",
"sqlglot>=23.0.2,<24",
"sqlparse>=0.4.4, <0.5",
"sqlparse>=0.4.4",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for a noob question, but should this be sqlparse>=0.5.0, to preclude people from using anything less?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not noob, I think it makes sense

@betodealmeida
Copy link
Member

@dpgaspar and @betodealmeida is the plan sitll—per [SIP-117] Improve SQL parsing —to remove sqlparse completely?

Yes, I'm still working on it. Had to put aside to work on catalogs (SIP-95), but I'm going to finish the work on SIP-117 soon.

@dpgaspar dpgaspar mentioned this pull request May 14, 2024
9 tasks
@dpgaspar dpgaspar requested a review from rusackas May 14, 2024 19:13
Copy link

codecov bot commented May 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.40%. Comparing base (76d897e) to head (fe92963).
Report is 1094 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #28144       +/-   ##
===========================================
+ Coverage   60.48%   83.40%   +22.92%     
===========================================
  Files        1931      521     -1410     
  Lines       76236    37467    -38769     
  Branches     8568        0     -8568     
===========================================
- Hits        46114    31251    -14863     
+ Misses      28017     6216    -21801     
+ Partials     2105        0     -2105     
Flag Coverage Δ
hive 49.09% <ø> (-0.07%) ⬇️
javascript ?
mysql 77.12% <ø> (?)
postgres 77.23% <ø> (?)
presto 53.65% <ø> (-0.15%) ⬇️
python 83.40% <ø> (+19.92%) ⬆️
sqlite 76.68% <ø> (?)
unit 58.81% <ø> (+1.19%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dpgaspar dpgaspar merged commit d7b6f1c into apache:master May 15, 2024
@dpgaspar dpgaspar deleted the fix/bump-sqlparse branch May 15, 2024 08:36
@denodo-research-labs
Copy link
Contributor

CVE-2024-4340 is reported as HIGH by some security tools like PRISMA.

Will this version bump be applied to the 3.1-x branch too to avoid that CVE affecting current stable versions?

@mistercrunch mistercrunch added the v3.1 Label added by the release manager to track PRs to be included in the 3.1 branch label May 20, 2024
@john-bodley john-bodley removed the review:checkpoint Last PR reviewed during the daily review standup label May 23, 2024
vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 4.1.0 First shipped in 4.1.0 labels Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS v3.1 Label added by the release manager to track PRs to be included in the 3.1 branch 🚢 4.1.0 First shipped in 4.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants