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

Elasticsearch does not handle some http methods properly #31017

Closed
Tim-Brooks opened this issue Jun 1, 2018 · 5 comments
Closed

Elasticsearch does not handle some http methods properly #31017

Tim-Brooks opened this issue Jun 1, 2018 · 5 comments
Assignees
Labels
>bug :Distributed/Network Http and internode communication implementations v6.4.0 v7.0.0-beta1

Comments

@Tim-Brooks
Copy link
Contributor

Currently when we convert a low-level netty http method to an elasticsearch http method we consider an unknown request type to be a GET request.

    public Method method() {
        HttpMethod httpMethod = request.method();
        if (httpMethod == HttpMethod.GET)
            return Method.GET;

        if (httpMethod == HttpMethod.POST)
            return Method.POST;

        if (httpMethod == HttpMethod.PUT)
            return Method.PUT;

        if (httpMethod == HttpMethod.DELETE)
            return Method.DELETE;

        if (httpMethod == HttpMethod.HEAD) {
            return Method.HEAD;
        }

        if (httpMethod == HttpMethod.OPTIONS) {
            return Method.OPTIONS;
        }

        return Method.GET;
    }

Currently that means that PATCH, TRACE, and CONNECT methods are treated as GET methods by elasticsearch. If we do not plan on supporting these methods, we should just return a 405 instead of silently pretending that these are GET requests.

@Tim-Brooks Tim-Brooks added >bug :Distributed/Network Http and internode communication implementations v7.0.0 labels Jun 1, 2018
@Tim-Brooks Tim-Brooks self-assigned this Jun 1, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jasontedor
Copy link
Member

That's not good!

@jasontedor
Copy link
Member

Also I think that we can and should backport any fix to 6.x, this is a bug indeed as you have already labeled.

Tim-Brooks added a commit to Tim-Brooks/elasticsearch that referenced this issue Jun 1, 2018
This is related to elastic#31017. That issue identified that these three http
methods were treated like GET requests. This commit adds them to
RestRequest. This means that these methods will be handled properly and
generate 405s.
@Tim-Brooks
Copy link
Contributor Author

This has been merged into master. It still needs to be back ported to 6.x.

Tim-Brooks added a commit that referenced this issue Jun 1, 2018
This is related to #31017. That issue identified that these three http
methods were treated like GET requests. This commit adds them to
RestRequest. This means that these methods will be handled properly and
generate 405s.
Tim-Brooks added a commit to Tim-Brooks/elasticsearch that referenced this issue Jun 4, 2018
This is related to elastic#31017. That issue identified that these three http
methods were treated like GET requests. This commit adds them to
RestRequest. This means that these methods will be handled properly and
generate 405s.
Tim-Brooks added a commit that referenced this issue Jun 4, 2018
This is related to #31017. That issue identified that these three http
methods were treated like GET requests. This commit adds them to
RestRequest. This means that these methods will be handled properly and
generate 405s.
@Tim-Brooks
Copy link
Contributor Author

The fix has been merged into master and 6.x. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed/Network Http and internode communication implementations v6.4.0 v7.0.0-beta1
Projects
None yet
Development

No branches or pull requests

4 participants