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

IndicesPutAliasRequest with empty Index argument creates an index rather than failing to create an alias #201

Closed
kbirk opened this issue Nov 18, 2020 · 6 comments · Fixed by #411

Comments

@kbirk
Copy link

kbirk commented Nov 18, 2020

If I do the following in my code:

req := esapi.IndicesPutAliasRequest{}
req.Name = "my-alias"
req.Index = nil

_, err := req.Do(context.Background(), client)

I expect to receive a 400 and the following error payload:

{
    "error": {
        "root_cause": [
            {
                "type": "illegal_argument_exception",
                "reason": "[indices] can't be empty"
            }
        ],
        "type": "illegal_argument_exception",
        "reason": "[indices] can't be empty"
    },
    "status": 400
}

Instead I get a 200 for a create index.

PUT http://elastic-search:9200/my-alias 200 OK 52ms
        « {"acknowledged":true,"shards_acknowledged":true,"index":"my-alias"}

Looking at the code in the Do method of the IndicesPutAliasRequest I don't understand how this is possible. What am I missing that would cause this create index fallback?

@karmi
Copy link
Contributor

karmi commented Nov 19, 2020

I did a little bit of snooping around, and it seems like the esapi package correctly builds an URL like //_aliases/my-alias, but once it goes through the setReqURL() method, the double slash trips up the code which sets up the host. Here's an isolated example at Go playground https://play.golang.org/p/eJ_6MSEzd0w, which demonstrates the issue.

I'm no longer maintaining the library, so hopefully somebody can have a look into a fix. My hunch is that this is gonna be tricky on the transport level, since it just depends on the Go standard library. Unfortunately, nil is a valid value for []string, so only some kind of extra input validation would be robust — and that's certainly a slippery slope...

@kbirk
Copy link
Author

kbirk commented Nov 19, 2020

That is really interesting. Thanks for the quick response and go-playground example.

@ghost
Copy link

ghost commented Jan 12, 2022

@Anaethelion tagging you as you seem to be an active contributor to the project - have you any idea if the input validation that @karmi spoke of was added to later versions? We are experiencing this error updating from 7.4.0 to 7.5.0. The error appears to have been introduced when the custom Request construction was replaced with the stdlib http.NewRequest, diff here. Indeed, as @karmi pointed out, setReqURL replaces the detected host of _aliases with the host of that set on the connection pool. I can create a scratch file with a reproduction of the error if needed, many thanks :)

@Anaethelion
Copy link
Contributor

I've been digging on this, and what happens is as follows :

IndicesPutAliasRequest successfully builds a path with value //_aliases/alias-name and then proceed to build a http.Request object with

req, err := newRequest(method, path.String(), r.Body)

The problem is in this method, http.NewRequest expects an url in a string, not only a path. It proceeds to parse the url thus believing the leading // is the end of the scheme and tries to extract credentials, sending _aliases into the Host field which is then replaced by setReqURL.

I'll see what I can do without breaking the API!

@ghost
Copy link

ghost commented Jan 14, 2022

@Anaethelion I came to the same solution, thanks for looking into it!

@ebuildy
Copy link

ebuildy commented Jan 21, 2022

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

Successfully merging a pull request may close this issue.

4 participants