Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ConnectionInfo improvements #3661

Merged
merged 6 commits into from
Apr 9, 2023
Merged

ConnectionInfo improvements #3661

merged 6 commits into from
Apr 9, 2023

Conversation

TranceLove
Copy link
Collaborator

@TranceLove TranceLove commented Dec 11, 2022 •
edited

Description

To tackle problems with FTP/FTPS/SSH URLs having complex characters, and over-complicated parsing logic in ConnectionInfo.

URL format is
<scheme>://<base64-encoded username>:<encrypted base64-encoded password>@<host>:<port><path, if specified>

And because of the changes in URL format, there will be incompatibilities with FTP/FTPS/SSH URLs currently stored in database. You will have to create them again...

Changes:

  • Decouple ConnectionInfo from NetCopyClientConnectionPool to individual class
  • Use proven regex to parse URI instead of mangling substrings all the time. Uri.parse() is not working for our URLs here, hence had to use yet another ways
  • Defer decrypting password until authentication callables

Issue tracker

Fixes #1347, fixes #2964, fixes #3636, fixes #3560, fixes #3739

Automatic tests

  • Added test cases

Manual tests

  • Done

Build tasks success

Successfully running following tasks on local:

  • ./gradlew assembledebug
  • ./gradlew spotlessCheck

@TranceLove
Copy link
Collaborator Author

TranceLove commented Dec 11, 2022 •
edited

TODO:

  • More testcases, covering query string and even more complicated URLs
  • Manual tests with true FTP/FTPS/SSH connections, as well as UI Drawer/Dialog interactions
  • Refactorings to make use of NetCopyConnectionInfo to reduce extra string mangling

@TranceLove TranceLove added Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Area-Ssh/Scp/Sftp Area-FTPClient Issues related to FTP client implementation. labels Dec 11, 2022
@TranceLove TranceLove force-pushed the feature/1347 branch 5 times, most recently from c1b2ad7 to b653065 Compare December 19, 2022 14:42
@TranceLove TranceLove force-pushed the feature/1347 branch 3 times, most recently from e4c22cc to e12adf2 Compare December 22, 2022 16:27
@TranceLove TranceLove force-pushed the feature/1347 branch 6 times, most recently from d7743fe to 977ae4f Compare December 31, 2022 13:48
@TranceLove TranceLove marked this pull request as ready for review December 31, 2022 14:15
@TranceLove TranceLove mentioned this pull request Jan 20, 2023
Closed
@TranceLove TranceLove force-pushed the feature/1347 branch 3 times, most recently from b3bae63 to c73c5f1 Compare January 22, 2023 08:26
@TranceLove
Copy link
Collaborator Author

TranceLove commented Jan 22, 2023 •
edited

Added handling of path names containing plus sign (+) in c73c5f1. Hope this can fix #3636 and #3560 as well.

@VishnuSanal VishnuSanal mentioned this pull request Jan 26, 2023
Closed
@VishalNehra
Copy link
Member

So initial loading seem to work on smb, however after I close and restart the app, and tap the connection again, it's not able to load the list.

023-01-30 01:49:33.713 25283-29073 c.a.f.a.a....esListTask com.amaze.filemanager.debug          W  [AsyncTask #4        ] Failed to load smb files for path: smb://demo:Abc@[email protected]/jcifs.smb.SmbException: NTLMv2 requires extended security (jcifs.smb.client.useExtendedSecurity must be true if jcifs.smb.lmCompatibility >= 3)
                                                                                                    	at jcifs.smb.NtlmPasswordAuthenticator.getSigningKey(NtlmPasswordAuthenticator.java:519)
                                                                                                    	at jcifs.smb.SmbSessionImpl.sessionSetupSMB1(SmbSessionImpl.java:890)
                                                                                                    	at jcifs.smb.SmbSessionImpl.sessionSetup(SmbSessionImpl.java:486)
                                                                                                    	at jcifs.smb.SmbSessionImpl.send(SmbSessionImpl.java:369)
                                                                                                    	at jcifs.smb.SmbSessionImpl.send(SmbSessionImpl.java:347)
                                                                                                    	at jcifs.smb.SmbTreeImpl.treeConnect(SmbTreeImpl.java:611)
                                                                                                    	at jcifs.smb.SmbTreeConnection.connectTree(SmbTreeConnection.java:614)
                                                                                                    	at jcifs.smb.SmbTreeConnection.connectHost(SmbTreeConnection.java:568)
                                                                                                    	at jcifs.smb.SmbTreeConnection.connectHost(SmbTreeConnection.java:489)
                                                                                                    	at jcifs.smb.SmbTreeConnection.connect(SmbTreeConnection.java:465)
                                                                                                    	at jcifs.smb.SmbTreeConnection.connectWrapException(SmbTreeConnection.java:426)
                                                                                                    	at jcifs.smb.SmbFile.ensureTreeConnected(SmbFile.java:559)
                                                                                                    	at jcifs.smb.SmbEnumerationUtil.doEnum(SmbEnumerationUtil.java:201)
                                                                                                    	at jcifs.smb.SmbEnumerationUtil.listFiles(SmbEnumerationUtil.java:279)
                                                                                                    	at jcifs.smb.SmbFile.listFiles(SmbFile.java:1213)
                                                                                                    	at com.amaze.filemanager.asynchronous.asynctasks.LoadFilesListTask.listSmb(LoadFilesListTask.java:599)
                                                                                                    	at com.amaze.filemanager.asynchronous.asynctasks.LoadFilesListTask.doInBackground(LoadFilesListTask.java:154)
                                                                                                    	at com.amaze.filemanager.asynchronous.asynctasks.LoadFilesListTask.doInBackground(LoadFilesListTask.java:85)
                                                                                                    	at android.os.AsyncTask$3.call(AsyncTask.java:394)
                                                                                                    	at java.util.concurrent.FutureTask.run(FutureTask.java:264)
                                                                                                    	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1137)
                                                                                                    	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:637)
                                                                                                    	at java.lang.Thread.run(Thread.java:1012)
2023-01-30 01:49:33.739 25283-25283 c.a.f.u.f.MainFragment  com.amaze.filemanager.debug          W  [main                ] Load list operation cancelled

@TranceLove
Remove JUnit test report until further notice
adca402
@TranceLove
ConnectionInfo improvements
25e9bc7 
- Decouple from NetCopyClientConnectionPool to individual class
- Use proven regex to parse URI instead of mangling substrings all the time
- Defer decrypting password until authentication callables

In design it can handle the path part in URL encoded string, but seems the regex is enough to grep the parts, so let it be.
@TranceLove
Add handling of plus sign (+) in path name
9d493ea 
With test cases, with test cases to prove TeamAmaze#2964 will work.

Fixes TeamAmaze#3560, fixes TeamAmaze#3636, fixes TeamAmaze#2964
@TranceLove
NetCopyClientUtils.deriveUriFrom() remove keypair argument
335edd6 
Fixes problem with dropping connection from drawer
@TranceLove
Test fixes
64328e6
@TranceLove TranceLove force-pushed the feature/1347 branch 6 times, most recently from 09596fc to 21f34d1 Compare February 23, 2023 16:17
@TranceLove
Copy link
Collaborator Author

TranceLove commented Feb 24, 2023 •
edited

This is still in WIP?

Still a few rough edges after countless nights struggling a problem writing loops using Flowable.fromIterable() with nullable results...

  • Fix Codacy complaints
  • Fix update SSH connection problems, cannot delete connection
  • Actual tests again
  • Code cleanup - want to remove HybridFileExt

@TranceLove
Copy link
Collaborator Author

After 8d0654c #3739 should be solved. Allow me a couple of nights more to finish the remaining tasks...

@TranceLove TranceLove force-pushed the feature/1347 branch 6 times, most recently from 72d11f0 to f391919 Compare February 28, 2023 15:00
@TranceLove TranceLove marked this pull request as ready for review February 28, 2023 15:03
@TranceLove
Changes per PR feedback
026ce89 
- Fix password authentication regressions
- Add routine to cleanup path as necessary, to prevent duplicate slashes display at path (TeamAmaze#3739)
@VishalNehra VishalNehra merged commit b3d49f5 into TeamAmaze:release/4.0 Apr 9, 2023
@VishalNehra VishalNehra mentioned this pull request Apr 10, 2023
Open
@TranceLove TranceLove mentioned this pull request Apr 30, 2023
Open
VishnuSanal added a commit that referenced this pull request Jun 14, 2023
@VishnuSanal
remove the workaround done to replace plus symbol (via #3661)
f11f6a9 
Signed-off-by: VishnuSanal <[email protected]>
@TranceLove TranceLove mentioned this pull request Oct 15, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VishalNehra VishalNehra VishalNehra approved these changes

@EmmanuelMess EmmanuelMess Awaiting requested review from EmmanuelMess

@VishnuSanal VishnuSanal Awaiting requested review from VishnuSanal

Assignees
No one assigned
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Area-FTPClient Issues related to FTP client implementation. Area-Ssh/Scp/Sftp
Projects
None yet
Milestone
No milestone
2 participants
@TranceLove @VishalNehra