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

[#14] Avoid "dot in path" hickup #108

Closed
wants to merge 1 commit into from

Conversation

hidden-primary-net
Copy link
Contributor

Do not interpret "." in path names as a file extension.

Do not interpret "." in path names as a file extension.
@khrt
Copy link
Owner

khrt commented Apr 28, 2021

Thanks for your merge request @hidden-primary-net, but I have to reject it.

This would be a patch which, in my opinion, does the job slightly better:

index f9b8bef..08d5da0 100644
--- a/lib/Raisin/Middleware/Formatter.pm
+++ b/lib/Raisin/Middleware/Formatter.pm
@@ -9,6 +9,7 @@ package Raisin::Middleware::Formatter;
 
 use parent 'Plack::Middleware';
 
+use File::Basename qw(fileparse);
 use HTTP::Status qw(:constants);
 use Plack::Request;
 use Plack::Response;
@@ -71,8 +72,8 @@ sub call {
 sub _accept_header_set { length(shift || '') }
 sub _path_has_extension {
     my $path = shift;
-    my @chunks = split /\./, $path;
-    scalar(@chunks) > 1;
+    my (undef, undef, $suffix) = fileparse($path, qr"\..[^.]*$");
+    $suffix;
 }
 
 sub negotiate_format {
diff --git a/t/unit/middleware/formatter.t b/t/unit/middleware/formatter.t
index c112062..51b7a8b 100644
--- a/t/unit/middleware/formatter.t
+++ b/t/unit/middleware/formatter.t
@@ -291,4 +291,33 @@ subtest 'format_from_header' => sub {
     }
 };
 
+subtest '_path_has_extension' => sub {
+    my @CASES = (
+        {
+            path => '/a/b/c.exe',
+            expected => '.exe',
+            message => 'extension',
+        },
+        {
+            path => '/a/b.at/c',
+            expected => '',
+            message => 'no extension',
+        },
+        {
+            path => '/',
+            expected => '',
+            message => 'slash',
+        },
+        {
+            path => '',
+            expected => '',
+            message => 'empty',
+        },
+    );
+
+    for my $c (@CASES) {
+        is Raisin::Middleware::Formatter::_path_has_extension($c->{path}), $c->{expected}, $c->{message};
+    }
+};
+
 done_testing;

I'll try to make a fix and a new release somewhere today, that might include a fix for #14 as well.

Thanks for your contribution either way!

@khrt khrt closed this Apr 28, 2021
@hidden-primary-net
Copy link
Contributor Author

Thank you very much 👍

@khrt
Copy link
Owner

khrt commented Apr 28, 2021

Related #109

@khrt
Copy link
Owner

khrt commented Apr 28, 2021

I've released version 0.91 which includes the fix for your particular problem. Must be available on CPAN shortly.

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