[Security Fix] Upgrade mysql connector to 8.2.0 #15408
Conversation
processing/src/main/java/org/apache/druid/utils/ConnectionUriUtils.java
Outdated
Show resolved
Hide resolved
| if (entry.getKey() != null) { | ||
| if(entry.getValue() == null) { |
There was a problem hiding this comment.
is this actually possible?
There was a problem hiding this comment.
Yes, it is possible. Like in this case where keyonly doesn't have any value associated with it.
| } | ||
| } | ||
| } | ||
| if (properties.isEmpty()) { |
There was a problem hiding this comment.
why would you throw an error if there are no properties associated with the connection string? Instead you should throw an error if properties is null for some reason.
There was a problem hiding this comment.
With the initialisation of properties. It cannot be null anymore, hence empty remains the only possible scenario when nothing could be extracted from the connection string.
mustajibmk
changed the title
mustajibmk
force-pushed
the
upgrade-mysql-connector
branch
from
ea2e267 to
cdfc075
Compare
| @@ -159,46 +159,6 @@ public boolean isEnforceAllowedProperties() | |||
| } | |||
| ); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testWhenInvalidUrlFormat() | |||
There was a problem hiding this comment.
is this no longer an invalid url format?
There was a problem hiding this comment.
The parser no longer checks if every component is in the expected format it only checks if the URL starts with one of the required schema i.e jdbc:mysql,jdbc:mysql:loadbalancer, etc. So we will not be able to determine if this is a valid/invalid URL just by using the parser.
The way to determine if the URL is valid is by creating a connection with the DB.
...tadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLConnectorTest.java
Outdated
Show resolved
Hide resolved
| connectionUri, | ||
| null | ||
| ); | ||
| Class<?> connectionUrlClass = Class.forName(MYSQL_CONNECTION_URL); |
There was a problem hiding this comment.
can you describe your logic in a bit more detail?
There was a problem hiding this comment.
Now the method
- Checks if the string is in acceptable schema
- Get the
list<hostInfo>obtained from the string - From the
hostInfoobtain the properties and return the key of each property
mustajibmk
force-pushed
the
upgrade-mysql-connector
branch
from
13e16e3 to
d59eca1
Compare
mustajibmk
force-pushed
the
upgrade-mysql-connector
branch
from
8048694 to
32d5c7a
Compare
|
@mustajibmk are you still working on this, i have similar work: #16024 (comment), my bad, i did not know you have already work on this. |
|
@abhishekagarwal87 and @cryptoe may i continue to work on this, seems there is no response from 3 months ago. |
|
Go ahead please
…On Fri, 15 Mar 2024 at 2:29 AM, AlbericByte ***@***.***> wrote:
@abhishekagarwal87 <https://github.com/abhishekagarwal87> and @cryptoe
<https://github.com/cryptoe> may i continue to work on this, seems there
is no response from 3 months ago.
and i have a similar pr : #16024 (comment)
<#16024 (comment)>
***@***.*** <https://github.com/mustajibmk>
—
Reply to this email directly, view it on GitHub
<#15408 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALIWUJJQDD2KKH4PB7ITJTYYIFUBAVCNFSM6AAAAAA7UBROZGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJYGQ3TINBQGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@AlbericByte let's continue |
|
@abhishekagarwal87 Let's shift to #16024 since @AlbericByte has also finished the work and I see all the CI checks are green. Still thanks the work from @mustajibmk |
Description
Upgrade MySql connector to 8.2.0 to fix security vulnerabilities.
Release note
Key changed/added classes in this PR
parseURLmethod incom.mysql.jdbc.NonRegisteringDriverpreviously used. Instead, we use thecom.mysql.cj.conf.ConnectionUrlParserwhich only checks if the schema of the string matches the prescribed format. The test cases related to string parsing are hence removed.This PR has: