[pve-devel] [PATCH 0/2] improve error message when accessing a directory

[pve-devel] [PATCH 0/2] improve error message when accessing a directory

[Nicolas Frey]

I ran into this when I accidentally went to the path /pve-docs/api-viewer
instead of /pve-docs/api-viewer/index.html, where I got this error:

`unable to detect content type at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 499.`

Which IMO was not very telling of the underlying mistake I had made.
This patch aims to improve this error message by checking if the
queried file is a directory and returning a more descriptive
message to the user.

I'm not sure (since I'm not too familiar with this codebase) if this
breaks any existing functionality I may have missed or was intended
behaviour, though I didn't find anything that would suggest this.

See also: https://perldoc.perl.org/IO::File#NOTE

Nicolas Frey (2):
  apiserver: fix typo
  apiserver: check if file is directory before opening

 src/PVE/APIServer/AnyEvent.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

-- 
2.47.3


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH 1/2] apiserver: fix typo

[Nicolas Frey]

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 src/PVE/APIServer/AnyEvent.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index 59bc184..36dee02 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -478,7 +478,7 @@ sub send_file_start {
 
             if (!($fh = $download->{fh})) {
                 my $path = $download->{path};
-                die "internal error: {download} returned but neither fh not path given\n"
+                die "internal error: {download} returned but neither fh nor path given\n"
                     if !$path;
                 sysopen($fh, "$path", O_NONBLOCK | O_RDONLY)
                     or die "open stream path '$path' for reading failed: $!\n";
-- 
2.47.3


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH 2/2] apiserver: check if file is directory before opening

[Nicolas Frey]

Add a check for if the file being accessed is a directory.
This improves the error relayed to the user, which - when mistakenly
trying to access a directory instead of a file - would return:

`unable to detect content type at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 499.`

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 src/PVE/APIServer/AnyEvent.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index 36dee02..3a7fa1b 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -1252,6 +1252,9 @@ sub handle_request {
                 my ($subdir, $file) = ($1, $2);
                 if (my $dir = $self->{dirs}->{$subdir}) {
                     my $filename = "$dir$file";
+                    if (-d $filename) {
+                        die "unable to open file '$filename' because it is a directory\n";
+                    }
                     my $fh = IO::File->new($filename)
                         || die "unable to open file '$filename' - $!\n";
                     send_file_start($self, $reqstate, { path => $filename });
-- 
2.47.3


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel