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

Add implicit '/' to directories #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

antonu17
Copy link

@antonu17 antonu17 commented Feb 6, 2019

Something like this to fix #22

Something like this to fix pottava#22
@cablespaghetti
Copy link

Thanks for this, it gets me a little closer to what I want out of this project. However if I enable directory listings, this feature no longer works. I really need to learn Go...but any chance you can extend this PR so it takes precedence over directory listings?

@cablespaghetti
Copy link

cablespaghetti commented Jul 5, 2019

Ah sorry, I've just realised the dockerfile was building from git and not my local checked out changes. Your patch may indeed do what I need if I can work out how to build in into an image.

@cablespaghetti
Copy link

This seems to do what I was looking for, but is probably terrible code...

diff --git a/main.go b/main.go
index 85e64dd..4867758 100644
--- a/main.go
+++ b/main.go
@@ -314,19 +314,20 @@ func awss3(w http.ResponseWriter, r *http.Request) {
                }
                path = link.URL + path[idx+12:]
        }
+       var pathToGet string
        if strings.HasSuffix(path, "/") {
-               if c.directoryListing {
-                       s3listFiles(w, r, c.s3Bucket, c.s3KeyPrefix+path)
-                       return
-               }
-               path += c.indexDocument
+               pathToGet = c.s3KeyPrefix+path+c.indexDocument
+       } else {
+               pathToGet = c.s3KeyPrefix+path
        }
-       obj, err := s3get(c.s3Bucket, c.s3KeyPrefix+path, rangeHeader)
+       obj, err := s3get(c.s3Bucket, pathToGet, rangeHeader)
        if aerr, ok := err.(awserr.Error); ok {
                switch aerr.Code() {
                case s3.ErrCodeNoSuchKey:
-                       path += "/" + c.indexDocument
-                       obj, err = s3get(c.s3Bucket, c.s3KeyPrefix+path, rangeHeader)
+                       if c.directoryListing {
+                               s3listFiles(w, r, c.s3Bucket, c.s3KeyPrefix+path)
+                               return
+                       }
                }
        }

@excavador
Copy link

  •           pathToGet = c.s3KeyPrefix+path+c.indexDocument
    

What about:

filepath.Join(c.s3KeyPrefix, path, c.indexDocument)

?

It's more safe from "//" point of view.
Of course, S3 allows you to have paths with double slash, but it's definitely not ordinary case.

@tkrop
Copy link

tkrop commented Mar 30, 2020

@antonu17 I like this very simple suggestion. It actually would solve a challenge running behind ngnix for me too. Sad, that it has not been merged.

@EvgeniGordeev
Copy link

EvgeniGordeev commented Apr 15, 2020

@pottava Any update on this PR? Is there anything left to do?

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.

Add implicit '/' to directories
5 participants