XContentBuilder.writeValue causes StackOverflowError given Path

[pickypg]

If XContentBuilder receives a Path object to write, then it results in a StackOverflowError because it runs into the Iterable check, which Path implements (Path implements Iterable<Path>).

[szroland]

This is correct, since one segment of the path is still a path, which is its own first segment, etc. Weird API, that.

Still, @pickypg, I'm curious where you stumbled upon this, e.g. if you could post the stack trace (until it starts calling itself recursively). Because I don't see much value serializing a Path object like this, it would not be useful as an array of path elements even if there was not this issue of endless recursion. Calling toString() on it and storing it as a string could be more useful.

Not sure XContentBuilder needs explicit support of Path, or rather whatever is calling should decide how they want to serialize paths, especially if the value is expected to be read back as a Path eventually.

[pickypg]

Hi @szroland, I agree that it's a slightly odd usage, but I came across it while integration testing against Elasticearch 1.6. As the Groovy client author, I had to update a bunch of my tests to include support for the new path.repo setting (amongst other things). This involved changing the path for integration tests for snapshots to use a "valid" temporary directory.

// Create the repository
PutRepositoryResponse putResponse = clusterAdminClient.putRepository {
    name repoName
    type "fs"
    settings {
        location = randomRepoPath()
    }
}

where randomRepoPath() returns a Path. Without going into too much detail, this indirectly/effectively calls builder.field("location", randomRepoPath()), which is making use of XContentBuilder field(String name, Object value) and eventually writeValue(Object value).

This can be easily worked around by calling randomRepoPath().toString() or, presumably even better, randomRepoPath().toAbsolutePath().toString(), but there's nothing implying a problem until you call it.

Not sure XContentBuilder needs explicit support of Path, or rather whatever is calling should decide how they want to serialize paths, especially if the value is expected to be read back as a Path eventually.

We do have a bunch of Path-based settings (particularly on master in ES 2.0+), which I think will expose this "problem" more over time, particularly when something else is doing the serialization for you.

[szroland]

I added a pull request with a possible implementation to have something concrete to review / discuss.

[jpountz]

Fixed.