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.

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

feat/samba #87

Merged
merged 1 commit into from
Jan 2, 2024
Merged

feat/samba #87

merged 1 commit into from
Jan 2, 2024

Conversation

brian-mulier-p
Copy link
Member

closes #23

Copy link
Member

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

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

Can you explain why you add all those libraries inside the Gradle config and why it's not possible to use the standard common-vfs library?

Doing so will add maintenance cost as we would need to follow the changes upstream and do a release each time it's released upstream.

@brian-mulier-p
Copy link
Member Author

I added some comments but I have no strong opinion on the commons-vfs2 fork, that was already on the branch from @tchiotludo but I think it was mostly for sandbox but as I no longer use that dependency maybe we should go back on that release 🤔.
For the newly added lib I added a comment but basically commons-vfs2 doesn't offer SMB protocol handling (it's only part of their sandbox right now, AKA experimental features which are on another lib). However even that experimental handling from Apache doesn't support SMB2 / 3 while it's what's recommended security wise so I believe it won't be able to be used if we don't use another lib that handles that.
The lib I added provides an extra-layer over commons-vfs2's sandbox to add SMB2/3 support

@loicmathieu
Copy link
Member

If sandbox is not needed, please use the official release

@brian-mulier-p
Copy link
Member Author

Done

Copy link
Member

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

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

LGTM

@brian-mulier-p brian-mulier-p merged commit 3512715 into master Jan 2, 2024
1 check passed
@brian-mulier-p brian-mulier-p deleted the feat/samba branch January 3, 2024 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Samba
3 participants