You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/07/12 11:50:51 UTC
[GitHub] [commons-vfs] garydgregory commented on a diff in pull request #272: SFTP: Memory leak because AbstractFileProvider#findFileSystem fails to detect equality of SFTP FileSystemOptions
garydgregory commented on code in PR #272:
URL: https://github.com/apache/commons-vfs/pull/272#discussion_r918874064
##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/FileSystemOptionsTest.java:
##########
@@ -92,4 +96,63 @@ public void testEqualsHashCodeAndCompareTo() {
assertEquals(expected.hashCode(), actual.hashCode());
}
+ @Test
+ public void testEqualsHashCodeAndCompareToWithSftpIdentityProviderMatch() {
+ for ( int mask = 0 ; mask < 8 ; mask++ ) {
+ assertSftpOptionsEqual(
+ (mask & 1) == 1 ? new File("/tmp/test.priv") : null,
+ (mask & 2) == 2 ? new File("/tmp/test.pub") : null,
+ (mask & 4) == 4 ? new byte[] {1,2,3} : null
+ );
+ }
+ }
+
+ @Test
+ public void testEqualsHashCodeAndCompareToWithSftpIdentityProviderMismatch() {
+ assertSftpOptionsNotEqual(
+ new File("/tmp/test.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3},
+ new File("/tmp/test1.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3}
+ );
+
+ assertSftpOptionsNotEqual(
+ new File("/tmp/test.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3},
Review Comment:
Refactor string constants that are reused into locals to make it obvious when the reuse is intended.
##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/FileSystemOptionsTest.java:
##########
@@ -92,4 +96,63 @@ public void testEqualsHashCodeAndCompareTo() {
assertEquals(expected.hashCode(), actual.hashCode());
}
+ @Test
+ public void testEqualsHashCodeAndCompareToWithSftpIdentityProviderMatch() {
+ for ( int mask = 0 ; mask < 8 ; mask++ ) {
+ assertSftpOptionsEqual(
+ (mask & 1) == 1 ? new File("/tmp/test.priv") : null,
+ (mask & 2) == 2 ? new File("/tmp/test.pub") : null,
+ (mask & 4) == 4 ? new byte[] {1,2,3} : null
+ );
+ }
+ }
+
+ @Test
+ public void testEqualsHashCodeAndCompareToWithSftpIdentityProviderMismatch() {
+ assertSftpOptionsNotEqual(
+ new File("/tmp/test.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3},
+ new File("/tmp/test1.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3}
+ );
+
+ assertSftpOptionsNotEqual(
+ new File("/tmp/test.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3},
+ new File("/tmp/test.priv"), new File("/tmp/test1.pub"), new byte[] {1,2,3}
+ );
+
+ assertSftpOptionsNotEqual(
+ new File("/tmp/test.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3},
+ new File("/tmp/test.priv"), new File("/tmp/test.pub"), new byte[] {1,2,4}
+ );
+ }
+
+ private static void assertSftpOptionsEqual(File privKey, File pubKey, byte[] passphrase) {
+ final SftpFileSystemConfigBuilder builder = SftpFileSystemConfigBuilder.getInstance();
+
+ final FileSystemOptions expected = new FileSystemOptions();
+ final IdentityInfo info1 = new IdentityInfo( privKey, pubKey, passphrase);
+ builder.setIdentityProvider(expected, info1 );
+
+ final FileSystemOptions actual = new FileSystemOptions();
+ final IdentityInfo info2 = new IdentityInfo( privKey, pubKey, passphrase);
+ builder.setIdentityProvider(actual,info2);
+
+ assertEquals( 0, expected.compareTo( actual ) );
+ assertEquals( expected.hashCode(), actual.hashCode() );
+ }
+
+ private static void assertSftpOptionsNotEqual(File privKey1, File pubKey1, byte[] passphrase1,
+ File privKey2, File pubKey2, byte[] passphrase2) {
+ final SftpFileSystemConfigBuilder builder = SftpFileSystemConfigBuilder.getInstance();
+
+ final FileSystemOptions expected = new FileSystemOptions();
+ final IdentityInfo info1 = new IdentityInfo( privKey1, pubKey1, passphrase1);
+ builder.setIdentityProvider(expected, info1 );
+
+ final FileSystemOptions actual = new FileSystemOptions();
+ final IdentityInfo info2 = new IdentityInfo( privKey2, pubKey2, passphrase2);
+ builder.setIdentityProvider(actual,info2);
+
+ assertNotEquals( 0, expected.compareTo( actual ) );
Review Comment:
Use the same formatting style as the rest of VFS: No spaces around parentheses.
##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/FileSystemOptionsTest.java:
##########
@@ -92,4 +96,63 @@ public void testEqualsHashCodeAndCompareTo() {
assertEquals(expected.hashCode(), actual.hashCode());
}
+ @Test
+ public void testEqualsHashCodeAndCompareToWithSftpIdentityProviderMatch() {
+ for ( int mask = 0 ; mask < 8 ; mask++ ) {
+ assertSftpOptionsEqual(
+ (mask & 1) == 1 ? new File("/tmp/test.priv") : null,
+ (mask & 2) == 2 ? new File("/tmp/test.pub") : null,
+ (mask & 4) == 4 ? new byte[] {1,2,3} : null
+ );
+ }
+ }
+
+ @Test
+ public void testEqualsHashCodeAndCompareToWithSftpIdentityProviderMismatch() {
+ assertSftpOptionsNotEqual(
+ new File("/tmp/test.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3},
+ new File("/tmp/test1.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3}
+ );
+
+ assertSftpOptionsNotEqual(
+ new File("/tmp/test.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3},
+ new File("/tmp/test.priv"), new File("/tmp/test1.pub"), new byte[] {1,2,3}
+ );
+
+ assertSftpOptionsNotEqual(
Review Comment:
IMO "Equal" -> "Equals".
##########
commons-vfs2/src/test/java/org/apache/commons/vfs2/FileSystemOptionsTest.java:
##########
@@ -92,4 +96,63 @@ public void testEqualsHashCodeAndCompareTo() {
assertEquals(expected.hashCode(), actual.hashCode());
}
+ @Test
+ public void testEqualsHashCodeAndCompareToWithSftpIdentityProviderMatch() {
+ for ( int mask = 0 ; mask < 8 ; mask++ ) {
+ assertSftpOptionsEqual(
+ (mask & 1) == 1 ? new File("/tmp/test.priv") : null,
+ (mask & 2) == 2 ? new File("/tmp/test.pub") : null,
+ (mask & 4) == 4 ? new byte[] {1,2,3} : null
+ );
+ }
+ }
+
+ @Test
+ public void testEqualsHashCodeAndCompareToWithSftpIdentityProviderMismatch() {
+ assertSftpOptionsNotEqual(
+ new File("/tmp/test.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3},
+ new File("/tmp/test1.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3}
+ );
+
+ assertSftpOptionsNotEqual(
+ new File("/tmp/test.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3},
+ new File("/tmp/test.priv"), new File("/tmp/test1.pub"), new byte[] {1,2,3}
+ );
+
+ assertSftpOptionsNotEqual(
+ new File("/tmp/test.priv"), new File("/tmp/test.pub"), new byte[] {1,2,3},
+ new File("/tmp/test.priv"), new File("/tmp/test.pub"), new byte[] {1,2,4}
+ );
+ }
+
+ private static void assertSftpOptionsEqual(File privKey, File pubKey, byte[] passphrase) {
+ final SftpFileSystemConfigBuilder builder = SftpFileSystemConfigBuilder.getInstance();
+
+ final FileSystemOptions expected = new FileSystemOptions();
+ final IdentityInfo info1 = new IdentityInfo( privKey, pubKey, passphrase);
+ builder.setIdentityProvider(expected, info1 );
+
+ final FileSystemOptions actual = new FileSystemOptions();
+ final IdentityInfo info2 = new IdentityInfo( privKey, pubKey, passphrase);
+ builder.setIdentityProvider(actual,info2);
+
+ assertEquals( 0, expected.compareTo( actual ) );
+ assertEquals( expected.hashCode(), actual.hashCode() );
+ }
+
+ private static void assertSftpOptionsNotEqual(File privKey1, File pubKey1, byte[] passphrase1,
Review Comment:
Use final for parameters.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org