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

The "s3fs.protocol" environment variable applies to both the S3 client and the Proxy. #719

Closed
stanakaj opened this issue Jun 12, 2023 · 2 comments · Fixed by #720
Closed
Labels
bug Something isn't working
Milestone

Comments

@stanakaj
Copy link
Contributor

Thank you for continuing to maintain the source code forked from Upplication/Amazon-S3-FileSystem-NIO2. Your efforts are greatly appreciated.

Bug Description

The "s3fs.protocol" environment variable applies to both the S3 client and the Proxy. Therefore, if the S3 client uses HTTPS and the Proxy uses HTTP, they must use unified protocol. This behavior can be problematic in environments where it is not possible to change the Proxy settings.

Expected Behavior

I would like to separate the configuration of S3 and Proxy protocols.

Environment

  • s3fs-nio version:1.0.1
  • OS: not relevant
  • JDK: not relevant

A patch for my environment.

Below is the patch I created for my development environment. Although it is a tricky modification, I only changed the S3Factory.class, which has a clear scope of effect. If the changes below are OK, I would like to create a pull request.

// In org.carlspring.cloud.storage.s3fs.S3Factory.java
    public static final String PROXY_PROTOCOL = "s3fs.proxy.protocol"; // add a new constant

    protected ProxyConfiguration getProxyConfiguration(final Properties props)
    {


        final ProxyConfiguration.Builder builder = ProxyConfiguration.builder();

        if (props.getProperty(PROXY_HOST) != null)
        {
            final String host = props.getProperty(PROXY_HOST);
            final String portStr = props.getProperty(PROXY_PORT);
            int port = -1;
            try
            {
                port = portStr != null ? Integer.parseInt(portStr, RADIX) : -1;
            }
            catch (final NumberFormatException e)
            {
                printWarningMessage(props, PROXY_PORT);
            }
// modification start
           // Calls the getEndpointUri method after setting the PROTOCOL property to the value of PROXY_PROTOCOL.
           final Properties propsCopy = new Properties();
           for (String key : props.stringPropertyNames()) {
               propsCopy.setProperty(key, props.getProperty(key));
           }

            if (propsCopy.getProperty(PROXY_PROTOCOL) != null)
            {
            	propsCopy.setProperty(PROTOCOL, props.getProperty(PROXY_PROTOCOL));
            }
            final URI uri = getEndpointUri(host, port, propsCopy);
// modification end
            builder.endpoint(uri);
        }
@stanakaj stanakaj added bug Something isn't working needs triage The issue needs to be triaged, before work can commence labels Jun 12, 2023
@carlspring
Copy link
Owner

Hi @stanakaj ,

Thank you for your kind words and for reporting this!

Would you like to open a pull request for it? We can then review it and have a closer look. When you make the fork, could you please make sure that we have committer rights to your fork as well? Thanks!

Thanks in advance!

@stanakaj
Copy link
Contributor Author

I have created a pull request. Please check it when you have time. Thanks.

@steve-todorov steve-todorov added this to the 1.0.2 milestone Jun 15, 2023
@steve-todorov steve-todorov removed the needs triage The issue needs to be triaged, before work can commence label Jul 25, 2023
steve-todorov added a commit that referenced this issue Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants