[PATCH 11 of 12] Win32: fixed ngx_fs_bsize() for symlinks
Sergey Kandaurov
pluknet at nginx.com
Fri Feb 24 10:40:17 UTC 2023
> On 23 Feb 2023, at 22:46, Maxim Dounin <mdounin at mdounin.ru> wrote:
>
> Hello!
>
> On Wed, Feb 22, 2023 at 08:01:15PM +0400, Sergey Kandaurov wrote:
>
>>> On 19 Feb 2023, at 21:23, Maxim Dounin <mdounin at mdounin.ru> wrote:
>>>
>>> Hello!
>>>
>>> On Fri, Feb 17, 2023 at 07:17:02PM +0400, Sergey Kandaurov wrote:
>>>
>>>>> On 13 Jan 2023, at 01:35, Maxim Dounin <mdounin at mdounin.ru> wrote:
>>>>>
>>>>> # HG changeset patch
>>>>> # User Maxim Dounin <mdounin at mdounin.ru>
>>>>> # Date 1673549010 -10800
>>>>> # Thu Jan 12 21:43:30 2023 +0300
>>>>> # Node ID be7eb9ec28dcbfdfd2e850befc8d051c0e4d46fd
>>>>> # Parent e62c8e9724ba68a698a2c3613edca73fe4e1c4ae
>>>>> Win32: fixed ngx_fs_bsize() for symlinks.
>>>>>
>>>>> Just a drive letter might not correctly represent file system being used,
>>>>> notably when using symlinks (as created by "mklink /d"). As such, instead
>>>>> of calling GetDiskFreeSpace() with just a drive letter, we now always
>>>>> use GetDiskFreeSpace() with full path.
>>>>>
>>>>> diff -r e62c8e9724ba -r be7eb9ec28dc src/os/win32/ngx_files.c
>>>>> --- a/src/os/win32/ngx_files.c Thu Jan 12 21:43:14 2023 +0300
>>>>> +++ b/src/os/win32/ngx_files.c Thu Jan 12 21:43:30 2023 +0300
>>>>> @@ -955,14 +955,8 @@ ngx_directio_off(ngx_fd_t fd)
>>>>> size_t
>>>>> ngx_fs_bsize(u_char *name)
>>>>> {
>>>>> - u_char root[4];
>>>>> u_long sc, bs, nfree, ncl;
>>>>>
>>>>> - if (name[2] == ':') {
>>>>> - ngx_cpystrn(root, name, 4);
>>>>> - name = root;
>>>>> - }
>>>>> -
>>>>
>>>> BTW, I wonder how this condition could be true.
>>>> Specifically, what name should represent in order to match.
>>>> I'm happy that it's leaving though.
>>>
>>> I tend to think that this actually never worked, and the original
>>> intention was to test name[1] instead.
>>>
>>> Updated commit log:
>>>
>>> : Win32: removed attempt to use a drive letter in ngx_fs_bsize().
>>> :
>>> : Just a drive letter might not correctly represent file system being used,
>>> : notably when using symlinks (as created by "mklink /d"). As such, instead
>>> : of trying to call GetDiskFreeSpace() with just a drive letter, we now always
>>> : use GetDiskFreeSpace() with full path.
>>> :
>>> : Further, it looks like the code to use just a drive letter never worked,
>>> : since it tried to test name[2] instead of name[1] to be ':'.
>>>
>>> [...]
>>>
>>
>> Looks fine, thanks.
>
> Thanks for the review, pushed to http://mdounin.ru/hg/nginx.
Tests for autoindex and dav modules for your consideration.
Largely based on existing tests.
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1677235010 -14400
# Fri Feb 24 14:36:50 2023 +0400
# Node ID 0e2ad324773111b2b2798f6926ce58a7ba93b30e
# Parent 4d44ea5ccca73813f557bda1fb4d024aaa4c9eea
Tests: win32 autoindex tests.
diff --git a/autoindex_win32.t b/autoindex_win32.t
new file mode 100644
--- /dev/null
+++ b/autoindex_win32.t
@@ -0,0 +1,119 @@
+#!/usr/bin/perl
+
+# (C) Sergey Kandaurov
+# (C) Nginx, Inc.
+
+# Tests for autoindex module.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+
+use Encode qw/ encode /;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+eval { require Win32API::File; };
+plan(skip_all => 'Win32API::File not installed') if $@;
+
+my $t = Test::Nginx->new()->has(qw/http autoindex charset/)->plan(9)
+ ->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+http {
+ %%TEST_GLOBALS_HTTP%%
+
+ server {
+ listen 127.0.0.1:8080;
+ server_name localhost;
+
+ location / {
+ autoindex on;
+ charset utf-8;
+ }
+ }
+}
+
+EOF
+
+my $d = $t->testdir();
+
+mkdir("$d/test-dir");
+$t->write_file('test-file', '');
+
+my $file = "$d/test-file-" . ("\x{043c}\x{0438}") x 3;
+win32_write_file($file, '');
+
+my $dir = "$d/test-dir-" . ("\x{043c}\x{0438}") x 3;
+win32_mkdir($dir);
+
+my $subfile = "$dir/test-subfile-" . ("\x{043c}\x{0438}") x 3;
+win32_write_file($subfile, '');
+
+$t->run();
+
+###############################################################################
+
+my $r = http_get('/');
+
+like($r, qr!href="test-file"!ms, 'file');
+like($r, qr!href="test-dir/"!ms, 'directory');
+
+TODO: {
+local $TODO = 'not yet' unless $t->has_version('1.23.4');
+
+like($r, qr!href="test-file-(%d0%bc%d0%b8){3}"!msi, 'utf file link');
+like($r, qr!test-file-(\xd0\xbc\xd0\xb8){3}</a>!ms, 'utf file name');
+
+like($r, qr!href="test-dir-(%d0%bc%d0%b8){3}/"!msi, 'utf dir link');
+like($r, qr!test-dir-(\xd0\xbc\xd0\xb8){3}/</a>!ms, 'utf dir name');
+
+$r = http_get('/test-dir-' . "\xd0\xbc\xd0\xb8" x 3 . '/');
+
+like($r, qr!Index of /test-dir-(\xd0\xbc\xd0\xb8){3}/!msi, 'utf subdir index');
+
+like($r, qr!href="test-subfile-(%d0%bc%d0%b8){3}"!msi, 'utf subdir link');
+like($r, qr!test-subfile-(\xd0\xbc\xd0\xb8){3}</a>!msi, 'utf subdir name');
+
+}
+
+###############################################################################
+
+sub win32_mkdir {
+ my ($name) = @_;
+
+ mkdir("$d/test-dir-tmp");
+ Win32API::File::MoveFileW(encode("UTF-16LE","$d/test-dir-tmp\0"),
+ encode("UTF-16LE", $name . "\0")) or die "$^E";
+}
+
+sub win32_write_file {
+ my ($name, $data) = @_;
+
+ my $h = Win32API::File::CreateFileW(encode("UTF-16LE", $name . "\0"),
+ Win32API::File::FILE_READ_DATA()
+ | Win32API::File::FILE_WRITE_DATA(), 0, [],
+ Win32API::File::CREATE_NEW(), 0, []) or die $^E;
+
+ Win32API::File::WriteFile($h, $data, 0, [], []) or die $^E;
+ Win32API::File::CloseHandle($h) or die $^E;
+}
+
+###############################################################################
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1677235049 -14400
# Fri Feb 24 14:37:29 2023 +0400
# Node ID 9378b768e0668dbe080d301818b394e71a2bb84d
# Parent 0e2ad324773111b2b2798f6926ce58a7ba93b30e
Tests: dav tests with UTF-8.
diff --git a/dav_utf8.t b/dav_utf8.t
new file mode 100644
--- /dev/null
+++ b/dav_utf8.t
@@ -0,0 +1,229 @@
+#!/usr/bin/perl
+
+# (C) Maxim Dounin
+# (C) Sergey Kandaurov
+# (C) Nginx, Inc.
+
+# Tests for nginx dav module with utf8 encoded names.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+
+use Encode qw/ encode /;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+eval { require Win32API::File; };
+plan(skip_all => 'Win32API::File not installed') if $@ && $^O eq 'MSWin32';
+
+my $t = Test::Nginx->new()->has(qw/http dav/)->plan(18);
+
+$t->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+http {
+ %%TEST_GLOBALS_HTTP%%
+
+ server {
+ listen 127.0.0.1:8080;
+ server_name localhost;
+
+ absolute_redirect off;
+
+ location / {
+ dav_methods PUT DELETE MKCOL COPY MOVE;
+ }
+
+ location /i/ {
+ alias %%TESTDIR%%/;
+ dav_methods PUT DELETE MKCOL COPY MOVE;
+ }
+ }
+}
+
+EOF
+
+$t->run();
+
+###############################################################################
+
+local $TODO = 'not yet' unless $t->has_version('1.23.4');
+
+my $r;
+
+my $file = "file-%D0%BC%D0%B8";
+my $file_path = "file-\x{043c}\x{0438}";
+
+$r = http(<<EOF . '0123456789');
+PUT /$file HTTP/1.1
+Host: localhost
+Connection: close
+Content-Length: 10
+
+EOF
+
+like($r, qr/201 Created.*(Content-Length|\x0d\0a0\x0d\x0a)/ms, 'put file');
+is(getfilesize($t->testdir() . '/' . $file_path), 10, 'put file size');
+
+$r = http(<<EOF);
+PUT /$file HTTP/1.1
+Host: localhost
+Connection: close
+Content-Length: 0
+
+EOF
+
+like($r, qr/204 No Content/, 'put file again');
+unlike($r, qr/Content-Length|Transfer-Encoding/, 'no length in 204');
+is(getfilesize($t->testdir() . '/' . $file_path), 0, 'put file again size');
+
+$r = http(<<EOF);
+DELETE /$file HTTP/1.1
+Host: localhost
+Connection: close
+Content-Length: 0
+
+EOF
+
+like($r, qr/204 No Content/, 'delete file');
+unlike($r, qr/Content-Length|Transfer-Encoding/, 'no length in 204');
+ok(!fileexists($t->testdir() . '/' . $file_path), 'file deleted');
+
+$r = http(<<EOF . '0123456789' . 'extra');
+PUT /$file HTTP/1.1
+Host: localhost
+Connection: close
+Content-Length: 10
+
+EOF
+
+like($r, qr/201 Created.*(Content-Length|\x0d\0a0\x0d\x0a)/ms,
+ 'put file extra data');
+is(getfilesize($t->testdir() . '/' . $file_path), 10,
+ 'put file extra data size');
+
+TODO: {
+local $TODO = 'not yet' unless $t->has_version('1.21.0');
+
+$r = http(<<EOF . '0123456789');
+PUT /$file%20sp HTTP/1.1
+Host: localhost
+Connection: close
+Content-Length: 10
+
+EOF
+
+like($r, qr!Location: /$file%20sp\x0d?$!ms, 'put file escaped');
+
+}
+
+$r = http(<<EOF);
+COPY /$file HTTP/1.1
+Host: localhost
+Destination: /$file-moved%20escape
+Connection: close
+
+EOF
+
+like($r, qr/204 No Content/, 'copy file escaped');
+is(getfilesize($t->testdir() . "/$file_path-moved escape"), 10,
+ 'file copied unescaped');
+
+my $dir_path = "dir-\x{043c}\x{0438}";
+my $dir = "dir-%D0%BC%D0%B8";
+
+# 201 replies contain body, response should indicate it's empty
+
+$r = http(<<EOF);
+MKCOL /$dir/ HTTP/1.1
+Host: localhost
+Connection: close
+
+EOF
+
+like($r, qr/201 Created.*(Content-Length|\x0d\0a0\x0d\x0a)/ms, 'mkcol');
+
+SKIP: {
+skip 'perl too old', 1 if !$^V or $^V lt v5.12.0;
+
+like($r, qr!(?(?{ $r =~ /Location/ })Location: /$dir/)!, 'mkcol location');
+
+}
+
+$r = http(<<EOF);
+COPY /$dir/ HTTP/1.1
+Host: localhost
+Destination: /$dir-moved/
+Connection: close
+
+EOF
+
+like($r, qr/201 Created.*(Content-Length|\x0d\0a0\x0d\x0a)/ms, 'copy dir');
+
+$r = http(<<EOF);
+MOVE /$dir/ HTTP/1.1
+Host: localhost
+Destination: /$dir-moved/
+Connection: close
+
+EOF
+
+like($r, qr/201 Created.*(Content-Length|\x0d\0a0\x0d\x0a)/ms, 'move dir');
+
+$r = http(<<EOF);
+DELETE /$dir-moved/ HTTP/1.1
+Host: localhost
+Connection: close
+
+EOF
+
+unlike($r, qr/200 OK.*Content-Length|Transfer-Encoding/ms, 'delete dir');
+
+###############################################################################
+
+sub getfilesize {
+ my ($path) = @_;
+
+ return -s $path if $^O ne 'MSWin32';
+
+ $path = encode("UTF-16LE", $path . "\0");
+ my $h = Win32API::File::CreateFileW($path,
+ Win32API::File::FILE_READ_DATA(), 0,[],
+ Win32API::File::OPEN_EXISTING(), 0, []) or return;
+ my $filesize = Win32API::File::getFileSize($h);
+ Win32API::File::CloseHandle($h) or die $^E;
+ return $filesize;
+}
+
+sub fileexists {
+ my ($path) = @_;
+
+ return -f $path if $^O ne 'MSWin32';
+
+ $path = encode("UTF-16LE", $path . "\0");
+ my $h = Win32API::File::CreateFileW($path,
+ Win32API::File::FILE_READ_DATA(), 0,[],
+ Win32API::File::OPEN_EXISTING(), 0, []) or return;
+ Win32API::File::CloseHandle($h) or die $^E;
+ return 1;
+}
+
+###############################################################################
--
Sergey Kandaurov
More information about the nginx-devel
mailing list