| Message ID | CAHhKpQ4UFhtfRhByRiAm6KPy=KAzttYzZADLfakbMwpsp5GjpA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Convert trailing spaces and periods in path components | expand |
What was the server for these tests? On 9/29/2020 1:08 PM, Boris Protopopov wrote: > Testing: > > Prior to the patch: > > % mount -v > … > //host/share/home on /tmp/diry type cifs (rw,relatime,vers=default,... > % ls -l /tmp/diry/tmp > total 0 > % mkdir /tmp/diry/tmp/DirWithTrailingDot. > % ls -l /tmp/diry/tmp/DirWithTrailingDot. > total 0 > % touch /tmp/diry/tmp/DirWithTrailingDot./file > touch: cannot touch ‘/tmp/diry/tmp/DirWithTrailingDot./file’: No such > file or directory > % mkdir /tmp/diry/tmp/DirWithTrailingDot./dir > mkdir: cannot create directory > ‘/tmp/diry/tmp/DirWithTrailingDot./dir’: No such file or directory > % find /tmp/diry/tmp/DirWithTrailingDot. > /tmp/diry/tmp/DirWithTrailingDot. > % find /tmp/diry/tmp/DirWithTrailingSpace\ > find: `/tmp/diry/tmp/DirWithTrailingSpace ': No such file or directory > % mkdir /tmp/diry/tmp/DirWithTrailingSpace\ > % ls -l /tmp/diry/tmp/DirWithTrailingSpace\ > total 0 > % touch /tmp/diry/tmp/DirWithTrailingSpace\ /file > touch: cannot touch ‘/tmp/diry/tmp/DirWithTrailingSpace /file’: No > such file or directory > % mkdir /tmp/diry/tmp/DirWithTrailingSpace\ /dir > mkdir: cannot create directory ‘/tmp/diry/tmp/DirWithTrailingSpace > /dir’: No such file or directory > > After the patch: > > % umount /tmp/diry > % modprobe -r cifs > # load the fix > % modprobe cifs > % mount -t cifs -o... //host/share/home /tmp/diry > ... > % mkdir /tmp/diry/tmp/DirWithTrailingSpace\ /dir > % touch /tmp/diry/tmp/DirWithTrailingSpace\ /file > % mkdir /tmp/diry/tmp/DirWithTrailingDot./dir > % touch /tmp/diry/tmp/DirWithTrailingDot./file > % find /tmp/diry/tmp/ > /tmp/diry/tmp/ > /tmp/diry/tmp/DirWithTrailingDot. > /tmp/diry/tmp/DirWithTrailingDot./dir > /tmp/diry/tmp/DirWithTrailingDot./file > /tmp/diry/tmp/DirWithTrailingSpace > /tmp/diry/tmp/DirWithTrailingSpace /dir > /tmp/diry/tmp/DirWithTrailingSpace /file > % rm -rf /tmp/diry/tmp/* > % find /tmp/diry/tmp/ > /tmp/diry/tmp/ > > ---------- Forwarded message --------- > From: Boris Protopopov <pboris@amazon.com> > Date: Wed, Sep 23, 2020 at 8:39 PM > Subject: [PATCH] Convert trailing spaces and periods in path components > To: > Cc: <linux-cifs@vger.kernel.org>, <samba-technical@lists.samba.org>, > Boris Protopopov <pboris@amazon.com> > > > When converting trailing spaces and periods in paths, do so > for every component of the path, not just the last component. > If the conversion is not done for every path component, then > subsequent operations in directories with trailing spaces or > periods (e.g. create(), mkdir()) will fail with ENOENT. This > is because on the server, the directory will have a special > symbol in its name, and the client needs to provide the same. > > Signed-off-by: Boris Protopopov <pboris@amazon.com> > --- > fs/cifs/cifs_unicode.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c > index 498777d859eb..9bd03a231032 100644 > --- a/fs/cifs/cifs_unicode.c > +++ b/fs/cifs/cifs_unicode.c > @@ -488,7 +488,13 @@ cifsConvertToUTF16(__le16 *target, const char > *source, int srclen, > else if (map_chars == SFM_MAP_UNI_RSVD) { > bool end_of_string; > > - if (i == srclen - 1) > + /** > + * Remap spaces and periods found at the end of every > + * component of the path. The special cases of '.' and > + * '..' do not need to be dealt with explicitly because > + * they are addressed in namei.c:link_path_walk(). > + **/ > + if ((i == srclen - 1) || (source[i+1] == '\\')) > end_of_string = true; > else > end_of_string = false; > -- > 2.18.4 > >
tentatively merged ... running the usual functional tests http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/399 On Tue, Sep 29, 2020 at 12:08 PM Boris Protopopov <boris.v.protopopov@gmail.com> wrote: > > Testing: > > Prior to the patch: > > % mount -v > … > //host/share/home on /tmp/diry type cifs (rw,relatime,vers=default,... > % ls -l /tmp/diry/tmp > total 0 > % mkdir /tmp/diry/tmp/DirWithTrailingDot. > % ls -l /tmp/diry/tmp/DirWithTrailingDot. > total 0 > % touch /tmp/diry/tmp/DirWithTrailingDot./file > touch: cannot touch ‘/tmp/diry/tmp/DirWithTrailingDot./file’: No such > file or directory > % mkdir /tmp/diry/tmp/DirWithTrailingDot./dir > mkdir: cannot create directory > ‘/tmp/diry/tmp/DirWithTrailingDot./dir’: No such file or directory > % find /tmp/diry/tmp/DirWithTrailingDot. > /tmp/diry/tmp/DirWithTrailingDot. > % find /tmp/diry/tmp/DirWithTrailingSpace\ > find: `/tmp/diry/tmp/DirWithTrailingSpace ': No such file or directory > % mkdir /tmp/diry/tmp/DirWithTrailingSpace\ > % ls -l /tmp/diry/tmp/DirWithTrailingSpace\ > total 0 > % touch /tmp/diry/tmp/DirWithTrailingSpace\ /file > touch: cannot touch ‘/tmp/diry/tmp/DirWithTrailingSpace /file’: No > such file or directory > % mkdir /tmp/diry/tmp/DirWithTrailingSpace\ /dir > mkdir: cannot create directory ‘/tmp/diry/tmp/DirWithTrailingSpace > /dir’: No such file or directory > > After the patch: > > % umount /tmp/diry > % modprobe -r cifs > # load the fix > % modprobe cifs > % mount -t cifs -o... //host/share/home /tmp/diry > ... > % mkdir /tmp/diry/tmp/DirWithTrailingSpace\ /dir > % touch /tmp/diry/tmp/DirWithTrailingSpace\ /file > % mkdir /tmp/diry/tmp/DirWithTrailingDot./dir > % touch /tmp/diry/tmp/DirWithTrailingDot./file > % find /tmp/diry/tmp/ > /tmp/diry/tmp/ > /tmp/diry/tmp/DirWithTrailingDot. > /tmp/diry/tmp/DirWithTrailingDot./dir > /tmp/diry/tmp/DirWithTrailingDot./file > /tmp/diry/tmp/DirWithTrailingSpace > /tmp/diry/tmp/DirWithTrailingSpace /dir > /tmp/diry/tmp/DirWithTrailingSpace /file > % rm -rf /tmp/diry/tmp/* > % find /tmp/diry/tmp/ > /tmp/diry/tmp/ > > ---------- Forwarded message --------- > From: Boris Protopopov <pboris@amazon.com> > Date: Wed, Sep 23, 2020 at 8:39 PM > Subject: [PATCH] Convert trailing spaces and periods in path components > To: > Cc: <linux-cifs@vger.kernel.org>, <samba-technical@lists.samba.org>, > Boris Protopopov <pboris@amazon.com> > > > When converting trailing spaces and periods in paths, do so > for every component of the path, not just the last component. > If the conversion is not done for every path component, then > subsequent operations in directories with trailing spaces or > periods (e.g. create(), mkdir()) will fail with ENOENT. This > is because on the server, the directory will have a special > symbol in its name, and the client needs to provide the same. > > Signed-off-by: Boris Protopopov <pboris@amazon.com> > --- > fs/cifs/cifs_unicode.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c > index 498777d859eb..9bd03a231032 100644 > --- a/fs/cifs/cifs_unicode.c > +++ b/fs/cifs/cifs_unicode.c > @@ -488,7 +488,13 @@ cifsConvertToUTF16(__le16 *target, const char > *source, int srclen, > else if (map_chars == SFM_MAP_UNI_RSVD) { > bool end_of_string; > > - if (i == srclen - 1) > + /** > + * Remap spaces and periods found at the end of every > + * component of the path. The special cases of '.' and > + * '..' do not need to be dealt with explicitly because > + * they are addressed in namei.c:link_path_walk(). > + **/ > + if ((i == srclen - 1) || (source[i+1] == '\\')) > end_of_string = true; > else > end_of_string = false; > -- > 2.18.4
---------- Forwarded message --------- From: Boris Protopopov <boris.v.protopopov@gmail.com> Date: Tue, Sep 29, 2020 at 1:51 PM Subject: Re: Fwd: [PATCH] Convert trailing spaces and periods in path components To: Tom Talpey <tom@talpey.com> Windows Server 2019, should have mentioned above. On Tue, Sep 29, 2020 at 1:23 PM Tom Talpey <tom@talpey.com> wrote: > > What was the server for these tests? > > On 9/29/2020 1:08 PM, Boris Protopopov wrote: > > Testing: > > > > Prior to the patch: > > > > % mount -v > > … > > //host/share/home on /tmp/diry type cifs (rw,relatime,vers=default,... > > % ls -l /tmp/diry/tmp > > total 0 > > % mkdir /tmp/diry/tmp/DirWithTrailingDot. > > % ls -l /tmp/diry/tmp/DirWithTrailingDot. > > total 0 > > % touch /tmp/diry/tmp/DirWithTrailingDot./file > > touch: cannot touch ‘/tmp/diry/tmp/DirWithTrailingDot./file’: No such > > file or directory > > % mkdir /tmp/diry/tmp/DirWithTrailingDot./dir > > mkdir: cannot create directory > > ‘/tmp/diry/tmp/DirWithTrailingDot./dir’: No such file or directory > > % find /tmp/diry/tmp/DirWithTrailingDot. > > /tmp/diry/tmp/DirWithTrailingDot. > > % find /tmp/diry/tmp/DirWithTrailingSpace\ > > find: `/tmp/diry/tmp/DirWithTrailingSpace ': No such file or directory > > % mkdir /tmp/diry/tmp/DirWithTrailingSpace\ > > % ls -l /tmp/diry/tmp/DirWithTrailingSpace\ > > total 0 > > % touch /tmp/diry/tmp/DirWithTrailingSpace\ /file > > touch: cannot touch ‘/tmp/diry/tmp/DirWithTrailingSpace /file’: No > > such file or directory > > % mkdir /tmp/diry/tmp/DirWithTrailingSpace\ /dir > > mkdir: cannot create directory ‘/tmp/diry/tmp/DirWithTrailingSpace > > /dir’: No such file or directory > > > > After the patch: > > > > % umount /tmp/diry > > % modprobe -r cifs > > # load the fix > > % modprobe cifs > > % mount -t cifs -o... //host/share/home /tmp/diry > > ... > > % mkdir /tmp/diry/tmp/DirWithTrailingSpace\ /dir > > % touch /tmp/diry/tmp/DirWithTrailingSpace\ /file > > % mkdir /tmp/diry/tmp/DirWithTrailingDot./dir > > % touch /tmp/diry/tmp/DirWithTrailingDot./file > > % find /tmp/diry/tmp/ > > /tmp/diry/tmp/ > > /tmp/diry/tmp/DirWithTrailingDot. > > /tmp/diry/tmp/DirWithTrailingDot./dir > > /tmp/diry/tmp/DirWithTrailingDot./file > > /tmp/diry/tmp/DirWithTrailingSpace > > /tmp/diry/tmp/DirWithTrailingSpace /dir > > /tmp/diry/tmp/DirWithTrailingSpace /file > > % rm -rf /tmp/diry/tmp/* > > % find /tmp/diry/tmp/ > > /tmp/diry/tmp/ > > > > ---------- Forwarded message --------- > > From: Boris Protopopov <pboris@amazon.com> > > Date: Wed, Sep 23, 2020 at 8:39 PM > > Subject: [PATCH] Convert trailing spaces and periods in path components > > To: > > Cc: <linux-cifs@vger.kernel.org>, <samba-technical@lists.samba.org>, > > Boris Protopopov <pboris@amazon.com> > > > > > > When converting trailing spaces and periods in paths, do so > > for every component of the path, not just the last component. > > If the conversion is not done for every path component, then > > subsequent operations in directories with trailing spaces or > > periods (e.g. create(), mkdir()) will fail with ENOENT. This > > is because on the server, the directory will have a special > > symbol in its name, and the client needs to provide the same. > > > > Signed-off-by: Boris Protopopov <pboris@amazon.com> > > --- > > fs/cifs/cifs_unicode.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c > > index 498777d859eb..9bd03a231032 100644 > > --- a/fs/cifs/cifs_unicode.c > > +++ b/fs/cifs/cifs_unicode.c > > @@ -488,7 +488,13 @@ cifsConvertToUTF16(__le16 *target, const char > > *source, int srclen, > > else if (map_chars == SFM_MAP_UNI_RSVD) { > > bool end_of_string; > > > > - if (i == srclen - 1) > > + /** > > + * Remap spaces and periods found at the end of every > > + * component of the path. The special cases of '.' and > > + * '..' do not need to be dealt with explicitly because > > + * they are addressed in namei.c:link_path_walk(). > > + **/ > > + if ((i == srclen - 1) || (source[i+1] == '\\')) > > end_of_string = true; > > else > > end_of_string = false; > > -- > > 2.18.4 > > > >
Steve French <smfrench@gmail.com> writes: > tentatively merged ... running the usual functional tests > http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/399 We need to make sure it works with samba smb1&smb2 posix extensions. In smb1 posix extensions the paths are sent with / and \ is a valid component (and no restrictions on file name endings). Cheers,
I agree with testing, but also, looking at the comments:
/*
* Convert 16 bit Unicode pathname to wire format from string in
current code
* page. Conversion may involve remapping up the six characters that
are
* only legal in POSIX-like OS (if they are present in the string).
Path
* names are little endian 16 bit Unicode on the wire
*/
int
cifsConvertToUTF16(__le16 *target, const char *source, int srclen,
const struct nls_table *cp, int map_chars)
{
int i, charlen;
int j = 0;
char src_char;
__le16 dst_char;
wchar_t tmp;
wchar_t *wchar_to; /* UTF-16 */
int ret;
unicode_t u;
if (map_chars == NO_MAP_UNI_RSVD)
return cifs_strtoUTF16(target, source, PATH_MAX, cp);
...
it would seem that as long as the correct map_chars is picked, it
should work correctly? The latter, if not automatically determined,
can be controlled with mount options, I believe.
Boris.
On Wed, Sep 30, 2020 at 5:25 AM Aurélien Aptel <aaptel@suse.com> wrote:
>
> Steve French <smfrench@gmail.com> writes:
> > tentatively merged ... running the usual functional tests
> > http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/399
>
> We need to make sure it works with samba smb1&smb2 posix extensions.
> In smb1 posix extensions the paths are sent with / and \ is a valid
> component (and no restrictions on file name endings).
>
> Cheers,
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3
> SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)
diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c index 498777d859eb..9bd03a231032 100644 --- a/fs/cifs/cifs_unicode.c +++ b/fs/cifs/cifs_unicode.c @@ -488,7 +488,13 @@ cifsConvertToUTF16(__le16 *target, const char *source, int srclen, else if (map_chars == SFM_MAP_UNI_RSVD) { bool end_of_string; - if (i == srclen - 1) + /** + * Remap spaces and periods found at the end of every + * component of the path. The special cases of '.' and + * '..' do not need to be dealt with explicitly because + * they are addressed in namei.c:link_path_walk(). + **/ + if ((i == srclen - 1) || (source[i+1] == '\\')) end_of_string = true; else