Forbid index name . and ..

[xuzha]

Fixes #13858

[nik9000]

@xuzha - it looks good to me. Did you verify @divanikus's report first? I trust @divanikus but I figure its good to verify things like this.

[rmuir]

other possibilities like .., /, and \ could probably cause the same issue.

[divanikus]

I also think that other cases are possible. Most dangerous could be ".." or something like that.
I'm running 1.7.1 if any.

[xuzha]

@nik9000 Thanks, I did verify this via TransportClient.
Our Rest API treat . and .. as current path and parents path.

I could create index with ., but when I tried to delete the index, ES give me the exception:

java.io.IOException: Could not remove the following files (in the order of attempts):
  ...core/data/elasticsearch/nodes/1/indices/.: java.nio.file.FileSystemException: /Users/Xu/workspace/elasticsearch/core/data/elasticsearch/nodes/1/indices/.: Invalid argument

    at org.apache.lucene.util.IOUtils.rm(IOUtils.java:295)
    at org.elasticsearch.env.NodeEnvironment.deleteIndexDirectoryUnderLock(NodeEnvironment.java:424)
    at org.elasticsearch.env.NodeEnvironment.deleteIndexDirectorySafe(NodeEnvironment.java:406)
    at org.elasticsearch.indices.IndicesService.deleteIndexStore(IndicesService.java:522)
    at org.elasticsearch.indices.IndicesService.removeIndex(IndicesService.java:429)
    at org.elasticsearch.indices.IndicesService.deleteIndex(IndicesService.java:471)
    at org.elasticsearch.indices.cluster.IndicesClusterStateService.deleteIndex(IndicesClusterStateService.java:797)
    at org.elasticsearch.indices.cluster.IndicesClusterStateService.applyDeletedIndices(IndicesClusterStateService.java:234)
    at org.elasticsearch.indices.cluster.IndicesClusterStateService.clusterChanged(IndicesClusterStateService.java:171)
    at org.elasticsearch.cluster.service.InternalClusterService$UpdateTask.run(InternalClusterService.java:493)
    at org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedEsThreadPoolExecutor.java:231)
    at org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedEsThreadPoolExecutor.java:194)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)
[2015-09-29 14:33:57,953][WARN ][indices                  ] [Zarek] [.] still pending deletes present for shards [[.]] - retrying
[2015-09-29 14:33:57,965][WARN ][indices                  ] [Zarek] [.] still pending deletes present for shards [[.]] - retrying
[2015-09-29 14:33:57,990][WARN ][indices                  ] [Zarek] [.] still pending deletes present for shards [[.]] - retrying
[2015-09-29 14:33:58,032][WARN ][indices                  ] [Zarek] [.] still pending deletes present for shards [[.]] - retrying
[2015-09-29 14:33:58,115][WARN ][indices                  ] [Zarek] [.] still pending deletes present for shards [[.]] - retrying
[2015-09-29 14:33:58,281][WARN ][indices                  ] [Zarek] [.] still pending deletes present for shards [[.]] - retrying
[2015-09-29 14:33:58,606][WARN ][indices                  ] [Zarek] [.] still pending deletes present for shards [[.]] - retrying
[2015-09-29 14:33:59,251][WARN ][indices                  ] [Zarek] [.] still pending deletes present for shards [[.]] - retrying
[2015-09-29 14:34:00,535][WARN ][indices                  ] [Zarek] [.] still pending deletes present for shards [[.]] - retrying

In any case, we should forbid name with . , .. and others. I just cause problems at file system

[spinscale]

I dug a little bit into this... my idea was to check in NodeEnvironment.deleteIndexDirectoryUnderLock(), which calls indexPaths(index) to get the different paths of an index. We could check in that method that resolving to an absolute path actually ends with the index name. Sth like

if (false == nodePaths[i].indicesPath.resolve(index.name()).toAbsolutePath().normalize().toString().endsWith(index.getName())) {
 throw some fancy exception here?
}

this would prevent any weird path there/is it sufficient?

Update: This is still not sufficient as I thought, sorry for the noise.

[xuzha]

Thanks @spinscale, I think we still need to forbid these . and .. index names, just because ES need to create a dir using index name.

[xuzha]

@nik9000, would you like to take another look ?
I'm not sure if this is appropriate I just forbid . and .. here. Or should we wait for a more concrete solution for #9059

[nik9000]

I'm fine with this as is - I don't think its strong enough but it gets us further and prevents some silly mistakes.

[nik9000]

Like - we should still think about preventing more bad index names in a more holistic way - but this isn't going to stop that.

[xuzha]

Like Nik said, this is definitely not strong enough, it just a start. If there is no objection, I would merge it in tomorrow.

[xuzha]

2.1 7edac7e
2.x 24f6809