Skip to content

Commit d756e66

Browse files
committed
Merge branch 'master' into no-copy-settings-for-you
* master: Fix line length violation in cache tests Add stricter geohash parsing (elastic#30376) Add failing test for core cache deadlock [DOCS] convert forcemerge snippet
2 parents d2288e4 + ec939dc commit d756e66

File tree

9 files changed

+167
-29
lines changed

9 files changed

+167
-29
lines changed

docs/CHANGELOG.asciidoc

+15
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,21 @@ Machine Learning::
178178

179179
* Account for gaps in data counts after job is reopened ({pull}30294[#30294])
180180

181+
Add validation that geohashes are not empty and don't contain unsupported characters ({pull}30376[#30376])
182+
183+
[[release-notes-6.3.1]]
184+
== Elasticsearch version 6.3.1
185+
186+
//[float]
187+
//=== New Features
188+
189+
//[float]
190+
//=== Enhancements
191+
192+
[float]
193+
=== Bug Fixes
194+
195+
Reduce the number of object allocations made by {security} when resolving the indices and aliases for a request ({pull}30180[#30180])
181196
Rollup::
182197
* Validate timezone in range queries to ensure they match the selected job when
183198
searching ({pull}30338[#30338])

docs/reference/indices/forcemerge.asciidoc

+2-1
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ deletes. Defaults to `false`. Note that this won't override the
4141
[source,js]
4242
--------------------------------------------------
4343
POST /kimchy/_forcemerge?only_expunge_deletes=false&max_num_segments=100&flush=true
44-
4544
--------------------------------------------------
45+
// CONSOLE
46+
// TEST[s/^/PUT kimchy\n/]
4647

4748
[float]
4849
[[forcemerge-multi-index]]

server/src/main/java/org/elasticsearch/common/geo/GeoHashUtils.java

+6
Original file line numberDiff line numberDiff line change
@@ -171,11 +171,17 @@ public static final String stringEncodeFromMortonLong(long hashedVal, final int
171171
* Encode to a morton long value from a given geohash string
172172
*/
173173
public static final long mortonEncode(final String hash) {
174+
if (hash.isEmpty()) {
175+
throw new IllegalArgumentException("empty geohash");
176+
}
174177
int level = 11;
175178
long b;
176179
long l = 0L;
177180
for(char c : hash.toCharArray()) {
178181
b = (long)(BASE_32_STRING.indexOf(c));
182+
if (b < 0) {
183+
throw new IllegalArgumentException("unsupported symbol [" + c + "] in geohash [" + hash + "]");
184+
}
179185
l |= (b<<((level--*5) + MORTON_OFFSET));
180186
if (level < 0) {
181187
// We cannot handle more than 12 levels

server/src/main/java/org/elasticsearch/common/geo/GeoPoint.java

+6-2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import org.elasticsearch.common.xcontent.ToXContentFragment;
2929
import org.elasticsearch.common.xcontent.XContentBuilder;
3030
import org.elasticsearch.ElasticsearchParseException;
31-
import org.elasticsearch.common.Strings;
3231

3332
import java.io.IOException;
3433
import java.util.Arrays;
@@ -126,7 +125,12 @@ public GeoPoint resetFromIndexableField(IndexableField field) {
126125
}
127126

128127
public GeoPoint resetFromGeoHash(String geohash) {
129-
final long hash = mortonEncode(geohash);
128+
final long hash;
129+
try {
130+
hash = mortonEncode(geohash);
131+
} catch (IllegalArgumentException ex) {
132+
throw new ElasticsearchParseException(ex.getMessage(), ex);
133+
}
130134
return this.reset(GeoHashUtils.decodeLatitude(hash), GeoHashUtils.decodeLongitude(hash));
131135
}
132136

server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java

+33-25
Original file line numberDiff line numberDiff line change
@@ -299,14 +299,7 @@ public Mapper parse(ParseContext context) throws IOException {
299299
if (token == XContentParser.Token.START_ARRAY) {
300300
// its an array of array of lon/lat [ [1.2, 1.3], [1.4, 1.5] ]
301301
while (token != XContentParser.Token.END_ARRAY) {
302-
try {
303-
parse(context, GeoUtils.parseGeoPoint(context.parser(), sparse));
304-
} catch (ElasticsearchParseException e) {
305-
if (ignoreMalformed.value() == false) {
306-
throw e;
307-
}
308-
context.addIgnoredField(fieldType.name());
309-
}
302+
parseGeoPointIgnoringMalformed(context, sparse);
310303
token = context.parser().nextToken();
311304
}
312305
} else {
@@ -326,42 +319,57 @@ public Mapper parse(ParseContext context) throws IOException {
326319
} else {
327320
while (token != XContentParser.Token.END_ARRAY) {
328321
if (token == XContentParser.Token.VALUE_STRING) {
329-
parse(context, sparse.resetFromString(context.parser().text(), ignoreZValue.value()));
322+
parseGeoPointStringIgnoringMalformed(context, sparse);
330323
} else {
331-
try {
332-
parse(context, GeoUtils.parseGeoPoint(context.parser(), sparse));
333-
} catch (ElasticsearchParseException e) {
334-
if (ignoreMalformed.value() == false) {
335-
throw e;
336-
}
337-
}
324+
parseGeoPointIgnoringMalformed(context, sparse);
338325
}
339326
token = context.parser().nextToken();
340327
}
341328
}
342329
}
343330
} else if (token == XContentParser.Token.VALUE_STRING) {
344-
parse(context, sparse.resetFromString(context.parser().text(), ignoreZValue.value()));
331+
parseGeoPointStringIgnoringMalformed(context, sparse);
345332
} else if (token == XContentParser.Token.VALUE_NULL) {
346333
if (fieldType.nullValue() != null) {
347334
parse(context, (GeoPoint) fieldType.nullValue());
348335
}
349336
} else {
350-
try {
351-
parse(context, GeoUtils.parseGeoPoint(context.parser(), sparse));
352-
} catch (ElasticsearchParseException e) {
353-
if (ignoreMalformed.value() == false) {
354-
throw e;
355-
}
356-
context.addIgnoredField(fieldType.name());
357-
}
337+
parseGeoPointIgnoringMalformed(context, sparse);
358338
}
359339
}
360340

361341
context.path().remove();
362342
return null;
363343
}
364344

345+
/**
346+
* Parses geopoint represented as an object or an array, ignores malformed geopoints if needed
347+
*/
348+
private void parseGeoPointIgnoringMalformed(ParseContext context, GeoPoint sparse) throws IOException {
349+
try {
350+
parse(context, GeoUtils.parseGeoPoint(context.parser(), sparse));
351+
} catch (ElasticsearchParseException e) {
352+
if (ignoreMalformed.value() == false) {
353+
throw e;
354+
}
355+
context.addIgnoredField(fieldType.name());
356+
}
357+
}
358+
359+
/**
360+
* Parses geopoint represented as a string and ignores malformed geopoints if needed
361+
*/
362+
private void parseGeoPointStringIgnoringMalformed(ParseContext context, GeoPoint sparse) throws IOException {
363+
try {
364+
parse(context, sparse.resetFromString(context.parser().text(), ignoreZValue.value()));
365+
} catch (ElasticsearchParseException e) {
366+
if (ignoreMalformed.value() == false) {
367+
throw e;
368+
}
369+
context.addIgnoredField(fieldType.name());
370+
}
371+
}
372+
365373
@Override
366374
protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
367375
super.doXContentBody(builder, includeDefaults, params);

server/src/test/java/org/elasticsearch/common/cache/CacheTests.java

+33
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package org.elasticsearch.common.cache;
2121

22+
import org.elasticsearch.common.unit.TimeValue;
2223
import org.elasticsearch.test.ESTestCase;
2324
import org.junit.Before;
2425

@@ -343,6 +344,38 @@ protected long now() {
343344
assertEquals(numberOfEntries, cache.stats().getEvictions());
344345
}
345346

347+
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/30428")
348+
public void testComputeIfAbsentDeadlock() throws BrokenBarrierException, InterruptedException {
349+
final int numberOfThreads = randomIntBetween(2, 32);
350+
final Cache<Integer, String> cache =
351+
CacheBuilder.<Integer, String>builder().setExpireAfterAccess(TimeValue.timeValueNanos(1)).build();
352+
353+
final CyclicBarrier barrier = new CyclicBarrier(1 + numberOfThreads);
354+
for (int i = 0; i < numberOfThreads; i++) {
355+
final Thread thread = new Thread(() -> {
356+
try {
357+
barrier.await();
358+
for (int j = 0; j < numberOfEntries; j++) {
359+
try {
360+
cache.computeIfAbsent(0, k -> Integer.toString(k));
361+
} catch (final ExecutionException e) {
362+
throw new AssertionError(e);
363+
}
364+
}
365+
barrier.await();
366+
} catch (final BrokenBarrierException | InterruptedException e) {
367+
throw new AssertionError(e);
368+
}
369+
});
370+
thread.start();
371+
}
372+
373+
// wait for all threads to be ready
374+
barrier.await();
375+
// wait for all threads to finish
376+
barrier.await();
377+
}
378+
346379
// randomly promote some entries, step the clock forward, then check that the promoted entries remain and the
347380
// non-promoted entries were removed
348381
public void testPromotion() {

server/src/test/java/org/elasticsearch/common/geo/GeoHashTests.java

+12-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.elasticsearch.common.geo;
2020

2121
import org.apache.lucene.geo.Rectangle;
22+
import org.elasticsearch.ElasticsearchParseException;
2223
import org.elasticsearch.test.ESTestCase;
2324

2425
/**
@@ -95,7 +96,17 @@ public void testLongGeohashes() {
9596
Rectangle expectedBbox = GeoHashUtils.bbox(geohash);
9697
Rectangle actualBbox = GeoHashUtils.bbox(extendedGeohash);
9798
assertEquals("Additional data points above 12 should be ignored [" + extendedGeohash + "]" , expectedBbox, actualBbox);
98-
9999
}
100100
}
101+
102+
public void testInvalidGeohashes() {
103+
IllegalArgumentException ex;
104+
105+
ex = expectThrows(IllegalArgumentException.class, () -> GeoHashUtils.mortonEncode("55.5"));
106+
assertEquals("unsupported symbol [.] in geohash [55.5]", ex.getMessage());
107+
108+
ex = expectThrows(IllegalArgumentException.class, () -> GeoHashUtils.mortonEncode(""));
109+
assertEquals("empty geohash", ex.getMessage());
110+
}
111+
101112
}

server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java

+47
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import static org.hamcrest.Matchers.instanceOf;
5050
import static org.hamcrest.Matchers.not;
5151
import static org.hamcrest.Matchers.notNullValue;
52+
import static org.hamcrest.Matchers.nullValue;
5253

5354
public class GeoPointFieldMapperTests extends ESSingleNodeTestCase {
5455

@@ -398,4 +399,50 @@ public void testNullValue() throws Exception {
398399
assertThat(defaultValue, not(equalTo(doc.rootDoc().getField("location").binaryValue())));
399400
}
400401

402+
public void testInvalidGeohashIgnored() throws Exception {
403+
String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
404+
.startObject("properties")
405+
.startObject("location")
406+
.field("type", "geo_point")
407+
.field("ignore_malformed", "true")
408+
.endObject()
409+
.endObject().endObject().endObject());
410+
411+
DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser()
412+
.parse("type", new CompressedXContent(mapping));
413+
414+
ParsedDocument doc = defaultMapper.parse(SourceToParse.source("test", "type", "1", BytesReference
415+
.bytes(XContentFactory.jsonBuilder()
416+
.startObject()
417+
.field("location", "1234.333")
418+
.endObject()),
419+
XContentType.JSON));
420+
421+
assertThat(doc.rootDoc().getField("location"), nullValue());
422+
}
423+
424+
425+
public void testInvalidGeohashNotIgnored() throws Exception {
426+
String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
427+
.startObject("properties")
428+
.startObject("location")
429+
.field("type", "geo_point")
430+
.endObject()
431+
.endObject().endObject().endObject());
432+
433+
DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser()
434+
.parse("type", new CompressedXContent(mapping));
435+
436+
MapperParsingException ex = expectThrows(MapperParsingException.class,
437+
() -> defaultMapper.parse(SourceToParse.source("test", "type", "1", BytesReference
438+
.bytes(XContentFactory.jsonBuilder()
439+
.startObject()
440+
.field("location", "1234.333")
441+
.endObject()),
442+
XContentType.JSON)));
443+
444+
assertThat(ex.getMessage(), equalTo("failed to parse"));
445+
assertThat(ex.getRootCause().getMessage(), equalTo("unsupported symbol [.] in geohash [1234.333]"));
446+
}
447+
401448
}

server/src/test/java/org/elasticsearch/index/search/geo/GeoPointParsingTests.java

+13
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,19 @@ public void testInvalidField() throws IOException {
175175
assertThat(e.getMessage(), is("field must be either [lat], [lon] or [geohash]"));
176176
}
177177

178+
public void testInvalidGeoHash() throws IOException {
179+
XContentBuilder content = JsonXContent.contentBuilder();
180+
content.startObject();
181+
content.field("geohash", "!!!!");
182+
content.endObject();
183+
184+
XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content));
185+
parser.nextToken();
186+
187+
Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(parser));
188+
assertThat(e.getMessage(), is("unsupported symbol [!] in geohash [!!!!]"));
189+
}
190+
178191
private XContentParser objectLatLon(double lat, double lon) throws IOException {
179192
XContentBuilder content = JsonXContent.contentBuilder();
180193
content.startObject();

0 commit comments

Comments
 (0)