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

ignore trailing slash in share path #611

Closed
wants to merge 1 commit into from

Conversation

bonifaido
Copy link

@bonifaido bonifaido commented May 8, 2024

Trailing slashes in share paths (like: /home/me/Share/) caused permission issues with shares for clients on iOS and on Android TV for me, but otherwise they work fine with plain old Samba.

Might not be the best place to handle this, I'm open for suggestions! :)

share->path_sz = strlen(share->path);
if (share->path_sz > 1 && share->path[share->path_sz - 1] == '/')
Copy link
Member

Choose a reason for hiding this comment

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

@bonifaido Thanks for your patch:) BTW, if share pathname is "/home/me/Share//" or "/home/me/Share////", there no problem with this patch ?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the late reply.

Most probably there is a problem in those cases, is there a function available to do this in the module or in the kernel?

Copy link
Member

@namjaejeon namjaejeon May 20, 2024

Choose a reason for hiding this comment

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

there is no function for this. You need to add simple while loop like the following one. I didn't test it. can you check it ?

while (share->path_sz > 1 && share->path[share->path_sz - 1] == '/') {
                  share->path[share->path_sz - 1] = '\0';
                  share->path_sz--;
}

Copy link
Member

Choose a reason for hiding this comment

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

And please make the patch using git command to add signoff-by tag and your real name and mail address.
and need to add prefix(ksmbd: ) and patch description to the patch.
See other patches.
namjaejeon@ef5e170

Copy link
Member

Choose a reason for hiding this comment

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

Let me know if you are busy. I need to apply this patch today or till tomorrow.

Copy link
Author

Choose a reason for hiding this comment

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

Doing it right now!

Copy link
Author

Choose a reason for hiding this comment

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

I have incorporated the requested change, tested on my pizero 2w, with iOS VLC and Android VLC, works fine! Thank you!

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for your test:) I have optimized the patch and apply it to next branch.(namjaejeon@3172144)
If there is a problem, Let me know. Thanks.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you very much! What is the target kernel release for this patch?

Copy link
Member

Choose a reason for hiding this comment

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

I will add stable tag to the patch. This patch will be applied to stable kernels(5.15, 6.1, 6.6, 6.9) and 6.10-rc1.

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.

2 participants