You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "sdedic (via GitHub)" <gi...@apache.org> on 2023/01/25 15:59:08 UTC
[GitHub] [netbeans] sdedic opened a new pull request, #5363: Proper escaping of TNS_ADMIN path. Select JDBC driver with defined implementation.
sdedic opened a new pull request, #5363:
URL: https://github.com/apache/netbeans/pull/5363
This PR fixes two issues with OCI database support:
The `Download Wallet` action writes a JDBC connection string with a pathname in it, but the pathname is not properly escaped. This is not an issue on MacOS / Linux, but on Windows that uses backslashes as dir separators, the backslashes in string content are interpreted as escapes by the JDBC driver, and the resulting path will be broken.
Second, the action tries to find the appropriate JDBC driver, but `JDBCDriverManager.getDefault().getDrivers()` call may reeturn half-configured drivers, that have no JAR files associated with them. This PR makes the action to prefer such drivers that define jars with the actual driver code.
The original behaviour had the effect of selecting a wrong driver, i.e. OracleThin driver (without JARs) was persisted while there was full Oracle jdbc driver present (that was configured with appropriate code jars).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on a diff in pull request #5363: Proper escaping of TNS_ADMIN path. Select JDBC driver with defined implementation.
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #5363:
URL: https://github.com/apache/netbeans/pull/5363#discussion_r1087829877
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/actions/DownloadWalletAction.java:
##########
@@ -140,6 +163,16 @@ public void actionPerformed(ActionEvent ev) {
});
}
+ static boolean isAvailable(JDBCDriver driver) {
+ URL[] urls = driver.getURLs();
+ for (int i = 0; i < urls.length; i++) {
Review Comment:
Addressed in c158a2ca0119
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #5363: Proper escaping of TNS_ADMIN path. Select JDBC driver with defined implementation.
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #5363:
URL: https://github.com/apache/netbeans/pull/5363#issuecomment-1404939984
If it needs to go into NB17, then yes - base branch of PR needs changing to delivery, and commit rebasing on top of delivery if necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on a diff in pull request #5363: Proper escaping of TNS_ADMIN path. Select JDBC driver with defined implementation.
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #5363:
URL: https://github.com/apache/netbeans/pull/5363#discussion_r1088655051
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/actions/DownloadWalletAction.java:
##########
@@ -105,7 +110,22 @@ public void actionPerformed(ActionEvent ev) {
if (p.getDbUser() != null && p.getDbPassword() != null) {
JDBCDriver[] drivers = JDBCDriverManager.getDefault().getDrivers("oracle.jdbc.OracleDriver"); //NOI18N
+ JDBCDriver jarsPresent = null;
+
if (drivers.length > 0) {
+
+ // prefer a driver that actually defines some JARs.
+ for (JDBCDriver d : drivers) {
+ if (isAvailable(d)) {
+ jarsPresent = d;
+ break;
+ }
+ }
+ if (jarsPresent == null) {
+ jarsPresent = drivers[0];
+ // PENDING: should be shown to the user ?
+ LOG.log(Level.WARNING, "Unable to find driver JARs for wallet {0}, using fallback driver: {1}", new Object[] { walletPath, jarsPresent.getName() });
+ }
Review Comment:
Addressed in 7ce696e
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on pull request #5363: Proper escaping of TNS_ADMIN path. Select JDBC driver with defined implementation.
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on PR #5363:
URL: https://github.com/apache/netbeans/pull/5363#issuecomment-1404886380
This will be also needed for vsnetbeans, I guess I should rebase to `delivery` (@neilcsmith-net) ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on pull request #5363: Proper escaping of TNS_ADMIN path. Select JDBC driver with defined implementation.
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on PR #5363:
URL: https://github.com/apache/netbeans/pull/5363#issuecomment-1405014989
(marked as NB17, will rebase after @jhorvath's review, should be clean)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] MartinBalin merged pull request #5363: Proper escaping of TNS_ADMIN path. Select JDBC driver with defined implementation.
Posted by "MartinBalin (via GitHub)" <gi...@apache.org>.
MartinBalin merged PR #5363:
URL: https://github.com/apache/netbeans/pull/5363
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on a diff in pull request #5363: Proper escaping of TNS_ADMIN path. Select JDBC driver with defined implementation.
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #5363:
URL: https://github.com/apache/netbeans/pull/5363#discussion_r1087965038
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/actions/DownloadWalletAction.java:
##########
@@ -105,7 +110,22 @@ public void actionPerformed(ActionEvent ev) {
if (p.getDbUser() != null && p.getDbPassword() != null) {
JDBCDriver[] drivers = JDBCDriverManager.getDefault().getDrivers("oracle.jdbc.OracleDriver"); //NOI18N
+ JDBCDriver jarsPresent = null;
+
if (drivers.length > 0) {
+
+ // prefer a driver that actually defines some JARs.
+ for (JDBCDriver d : drivers) {
+ if (isAvailable(d)) {
+ jarsPresent = d;
+ break;
+ }
+ }
+ if (jarsPresent == null) {
+ jarsPresent = drivers[0];
+ // PENDING: should be shown to the user ?
+ LOG.log(Level.WARNING, "Unable to find driver JARs for wallet {0}, using fallback driver: {1}", new Object[] { walletPath, jarsPresent.getName() });
+ }
Review Comment:
For NB IDE usage, it's not an issue, as driver's jars can be configured when Connect... action is used. But yes, I can probably display an info dialog - but still create the connection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lbownik commented on a diff in pull request #5363: Proper escaping of TNS_ADMIN path. Select JDBC driver with defined implementation.
Posted by "lbownik (via GitHub)" <gi...@apache.org>.
lbownik commented on code in PR #5363:
URL: https://github.com/apache/netbeans/pull/5363#discussion_r1087723168
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/actions/DownloadWalletAction.java:
##########
@@ -140,6 +163,16 @@ public void actionPerformed(ActionEvent ev) {
});
}
+ static boolean isAvailable(JDBCDriver driver) {
+ URL[] urls = driver.getURLs();
+ for (int i = 0; i < urls.length; i++) {
Review Comment:
how about using for-each loop?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on pull request #5363: Proper escaping of TNS_ADMIN path. Select JDBC driver with defined implementation.
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on PR #5363:
URL: https://github.com/apache/netbeans/pull/5363#issuecomment-1406123008
Rebased onto `delivery`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jhorvath commented on a diff in pull request #5363: Proper escaping of TNS_ADMIN path. Select JDBC driver with defined implementation.
Posted by "jhorvath (via GitHub)" <gi...@apache.org>.
jhorvath commented on code in PR #5363:
URL: https://github.com/apache/netbeans/pull/5363#discussion_r1087937860
##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/actions/DownloadWalletAction.java:
##########
@@ -105,7 +110,22 @@ public void actionPerformed(ActionEvent ev) {
if (p.getDbUser() != null && p.getDbPassword() != null) {
JDBCDriver[] drivers = JDBCDriverManager.getDefault().getDrivers("oracle.jdbc.OracleDriver"); //NOI18N
+ JDBCDriver jarsPresent = null;
+
if (drivers.length > 0) {
+
+ // prefer a driver that actually defines some JARs.
+ for (JDBCDriver d : drivers) {
+ if (isAvailable(d)) {
+ jarsPresent = d;
+ break;
+ }
+ }
+ if (jarsPresent == null) {
+ jarsPresent = drivers[0];
+ // PENDING: should be shown to the user ?
+ LOG.log(Level.WARNING, "Unable to find driver JARs for wallet {0}, using fallback driver: {1}", new Object[] { walletPath, jarsPresent.getName() });
+ }
Review Comment:
Shouldn't we display message that the driver is not installed here? Driver with no jars is useless here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists