bcuff / elasticsearch-net-aws

Add-on to Elasticsearch.Net & NEST for using AWS's elasticsearch service.
Apache License 2.0
72 stars 27 forks source link

I think it breaks if I EnableHttpCompression() on the ConnectionSettings #36

Closed jakejscott closed 2 years ago

jakejscott commented 6 years ago

The signatures end up not being the same

bcuff commented 5 years ago

Closing this (housekeeping)

jp-cai commented 3 years ago

This seems like a legitimate issue. When I enable compression, AWS tells me that my signature doesn't match. Everything works fine until I turn compression on.

Am I just missing something obvious? Is this a known problem?

bcuff commented 3 years ago

@jp-cai can you provide:

This will help me reproduce the problem. Thanks!

jp-cai commented 3 years ago

Sure! This is in a .NET Core 3.1 application, using NEST + Elasticsearch.Net v7.8.2 and Elasticsearch.Net.Aws v7.0.6.

The issue occurs both running directly on my Windows machine (dotnet --version 3.1.402), and in a Linux container (mcr.microsoft.com/dotnet/core/aspnet:3.1).

As mentioned, this all works fine with EnableHttpCompression(false);

        private static IElasticClient BuildElasticClient(Uri uri)
        {
            uri.MustNotBeNull(() => throw new InvalidConfigurationException("Elasticsearch:Uri cannot be null."));

            var awsConnection = new AwsHttpConnection();

            var connectionSettings = new ConnectionSettings(
                new SingleNodeConnectionPool(uri),
                sourceSerializer: (builtin, settings) => new JsonNetSerializer(
                    builtin, settings,
                    () => new JsonSerializerSettings { NullValueHandling = NullValueHandling.Ignore }
                ),
                connection: awsConnection
            );

            //todo: this seems to result in an invalid signature
            connectionSettings.EnableHttpCompression();
            connectionSettings.ThrowExceptions();
            connectionSettings.DefaultFieldNameInferrer(x => x);
#if DEBUG
            connectionSettings.EnableDebugMode().PrettyJson();
#endif

            var client = new ElasticClient(connectionSettings);

            return client;
        }
bcuff commented 3 years ago

Thanks. I'll take a look and get back to you.

bcuff commented 3 years ago

@jp-cai sorry it took so long to get back to you. I attempted to solve the problem with this PR. However I was unable to get this to work with an AWS elasticsearch instance I was targeting. I did get past the signing errors but then ran into a 500 instead. I opened a support case with AWS and they weren't much help. Can you try compiling this yourself and seeing if it works against your target Elasticsearch service domain?

skovalyova commented 3 years ago

Hi, we are experiencing the same issue using .NET 5 + Elasticsearch.Net.Aws 7.1. When HTTP compression is enabled in NEST configuration and in an Elasticsearch cluster, the following error occurs: {"message":"The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details."}

Any updates on it?

bcuff commented 3 years ago

@skovalyova this PR was supposed to address the issue. I gets past the signing error but I end up getting a 500 on the version of AWS ESS that I was using. If you have the time/inclination you can try it out on your domain. If it works for you let me know.

I didn't have much luck with AWS support but I haven't tried it on the latest 7.x+ versions so it's worth a shot.

DenysVyskrebetsTR commented 2 years ago

Actually this code (with PR) works for my team right now AWS Lambda + dotnet 3.1 + AWS Elasticsearch with compression on

But We're experiencing stack overflow exception in aws and I believe that this library is a reason for that We're trying to find that part of the code that is a reason for SO

update: I think dispose pattern is not implemented correctly in SigningHttpMessageHandler. After we implemented as it should be (https://docs.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose#implement-the-dispose-pattern-for-a-derived-class) SO exception disappeared

bcuff commented 2 years ago

Merged the PR and published 7.2.0. I also addressed the potential stack overflow that was pointed out in the Dispose in 9186e736460948274067ef0c5f852d7980100901

I'll close this issue for now. Feel free to reopen if there are still problems. Thanks for all the feedback!

bcuff commented 2 years ago

Update. 7.2.0 had a bug that still caused the stack overflow. Please use 7.2.1