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