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

Make allow-directory-listing and provide-index-page compatible #112

Closed
johanvdw opened this issue Mar 8, 2023 · 11 comments
Closed

Make allow-directory-listing and provide-index-page compatible #112

johanvdw opened this issue Mar 8, 2023 · 11 comments

Comments

@johanvdw
Copy link

johanvdw commented Mar 8, 2023

I wonder if it is somehow possible to have directory listings in case the index.html page does not exist. I realise that in that case two requests to s3 have to be made, either first the page and then the listing if that fails, or first the listing and then the page if it is in the list.

@dekobon
Copy link
Collaborator

dekobon commented Mar 14, 2023

Hi @johanvdw, even if we wanted to do two requests to s3, I don't think we can do it with the current implementation of nginx and njs. If we make a s3 gateway binary module, then it may be possible.

@dekobon dekobon closed this as completed Mar 14, 2023
@ajschmidt8
Copy link
Contributor

ajschmidt8 commented May 17, 2023

@dekobon, I also have a use case where I need both ALLOW_DIRECTORY_LIST and PROVIDE_INDEX_PAGE to be enabled simultaneously.

Is there any way we could make this work?

I'm happy to open a PR if we can agree on an approach.

@dekobon
Copy link
Collaborator

dekobon commented May 17, 2023

There may be a way now. Njs now supports XML

So, you could parse the output of the directory list results and determine if there are no results. Basically, you would rewrite the logic here.

Then, one may could use fetch to get the index page. When doing that, you may actually want to fetch the index page by querying the S3 gateway itself over localhost so that the aws auth creds are embedded.

@ajschmidt8
Copy link
Contributor

Ok. I will try to take a look at this when time permits.

Please feel free to leave additional feedback on the approach if anything comes to mind.

In the meantime, can you re-open this issue, @dekobon?

@dekobon dekobon reopened this May 18, 2023
@AyodeAwe
Copy link
Contributor

Hi @dekobon, there might be an issue with the mentioned approach. I found the following in the docs:

As the js_body_filter handler returns its result immediately, it supports only synchronous operations. Thus, asynchronous operations such as r.subrequest() or setTimeout() are not supported.

It is not immediately clear how the fetch (being an async function) should be implemented within the logic here given this limitation.

Could you provide additional feedback?

@dekobon
Copy link
Collaborator

dekobon commented May 23, 2023

I wonder if you could modify it to a redirect of the location of the index page?

@AyodeAwe
Copy link
Contributor

Hmm, seems like r.internalRedirect simply gets ignored when called in js_body_filter. I created an issue in the njs repo describing this.

nginx/njs#645

@ajschmidt8
Copy link
Contributor

@dekobon, it looks like @AyodeAwe has opened a PR for this #139.

Can you take a look when you have a moment?

He uses a combination of js_content and ngx.fetch to see if an index.html page exists for a specific directory. If it does, he loads it. Otherwise, he loads the directory listing.

@dekobon
Copy link
Collaborator

dekobon commented Jun 9, 2023

Please see my comments on the PR.

@ajschmidt8
Copy link
Contributor

This issue can be closed now that #139 is merged.

Though we will likely open a subsequent PR to switch the following lines to use a HEAD request once this njs issue is resolved:

let reply = await ngx.fetch(
`http://127.0.0.1:80${url}`
);

@dekobon
Copy link
Collaborator

dekobon commented Jul 26, 2023

Thank you @ajschmidt8 !

Resolved in PR #139

@dekobon dekobon closed this as completed Jul 26, 2023
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

No branches or pull requests

4 participants