Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Add implicit '/' to directories #23

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

Add implicit '/' to directories #23

wants to merge 1 commit into from

Conversation

antonu17
Copy link

@antonu17 antonu17 commented Feb 6, 2019

Something like this to fix #22

@antonu17
Add implicit '/' to directories
31c9331 
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 •
edited
Loading

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 •
edited
Loading

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

@patrickdk77 patrickdk77 mentioned this pull request Jan 18, 2022
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add implicit '/' to directories
5 participants
@antonu17 @cablespaghetti @excavador @tkrop @EvgeniGordeev