From d4058de42073f13c4a047524eac8b11877c72fd7 Mon Sep 17 00:00:00 2001 From: Darren Foong Date: Sun, 15 Dec 2019 21:05:57 +0800 Subject: [PATCH 01/10] Add extractWarningValueFromWarningHeader from DeprecationLogger to Response to improve performance --- .../org/elasticsearch/client/Response.java | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java index c5a159eb39da5..bc01609cb1452 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/Response.java +++ b/client/rest/src/main/java/org/elasticsearch/client/Response.java @@ -116,19 +116,36 @@ public HttpEntity getEntity() { "GMT" + // GMT "\")?"); // closing quote (optional, since an older version can still send a warn-date) + /** + * Refer to org.elasticsearch.common.logging.DeprecationLogger + */ + private static String extractWarningValueFromWarningHeader(final String s) { + final int firstQuote = s.indexOf('\"'); + final int lastQuote = s.length() - 1; + final String warningValue = s.substring(firstQuote + 1, lastQuote); + assert assertWarningValue(s, warningValue); + return warningValue; + } + + /** + * Refer to org.elasticsearch.common.logging.DeprecationLogger + */ + private static boolean assertWarningValue(final String s, final String warningValue) { + final Matcher matcher = WARNING_HEADER_PATTERN.matcher(s); + final boolean matches = matcher.matches(); + assert matches; + return matcher.group(1).equals(warningValue); + } + /** * Returns a list of all warning headers returned in the response. */ public List getWarnings() { List warnings = new ArrayList<>(); for (Header header : response.getHeaders("Warning")) { - String warning = header.getValue(); - final Matcher matcher = WARNING_HEADER_PATTERN.matcher(warning); - if (matcher.matches()) { - warnings.add(matcher.group(1)); - } else { - warnings.add(warning); - } + final String warning = header.getValue(); + final String warningHeaderValue = extractWarningValueFromWarningHeader(warning); + warnings.add(warningHeaderValue); } return warnings; } From 7e9436849cb1684e581d9c909335f264ebc5a11c Mon Sep 17 00:00:00 2001 From: Darren Foong Date: Mon, 16 Dec 2019 08:01:14 +0800 Subject: [PATCH 02/10] Add matchWarningHeaderPatternByPrefix --- .../org/elasticsearch/client/Response.java | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java index bc01609cb1452..8dde767c8bff2 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/Response.java +++ b/client/rest/src/main/java/org/elasticsearch/client/Response.java @@ -116,6 +116,18 @@ public HttpEntity getEntity() { "GMT" + // GMT "\")?"); // closing quote (optional, since an older version can still send a warn-date) + /** + * Tests if a string matches the RFC 7234 specification for warning headers. + * This assumes that the warn code is always 299 and the warn agent is always + * Elasticsearch. + * + * @param s + * @return {@code true} if the input string matches the specification + */ + private static boolean matchWarningHeaderPatternByPrefix(final String s) { + return s.startsWith("299 Elasticsearch-"); + } + /** * Refer to org.elasticsearch.common.logging.DeprecationLogger */ @@ -144,8 +156,11 @@ public List getWarnings() { List warnings = new ArrayList<>(); for (Header header : response.getHeaders("Warning")) { final String warning = header.getValue(); - final String warningHeaderValue = extractWarningValueFromWarningHeader(warning); - warnings.add(warningHeaderValue); + if (matchWarningHeaderPatternByPrefix(warning)) { + warnings.add(extractWarningValueFromWarningHeader(warning)); + } else { + warnings.add(warning); + } } return warnings; } From d507d2d9a3f475b3671d103ae71f80599eda6d35 Mon Sep 17 00:00:00 2001 From: Darren Foong Date: Mon, 16 Dec 2019 08:10:05 +0800 Subject: [PATCH 03/10] Fix Javadoc --- .../rest/src/main/java/org/elasticsearch/client/Response.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java index 8dde767c8bff2..89f456763bab4 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/Response.java +++ b/client/rest/src/main/java/org/elasticsearch/client/Response.java @@ -121,7 +121,7 @@ public HttpEntity getEntity() { * This assumes that the warn code is always 299 and the warn agent is always * Elasticsearch. * - * @param s + * @param s the value of a warning header formatted according to RFC 7234 * @return {@code true} if the input string matches the specification */ private static boolean matchWarningHeaderPatternByPrefix(final String s) { From f7ac690c3fc49938d276606cd782fd34dab041b6 Mon Sep 17 00:00:00 2001 From: Darren Foong Date: Mon, 16 Dec 2019 10:09:57 +0800 Subject: [PATCH 04/10] Update matchWarningHeaderPatternByPrefix() to strip dates --- .../org/elasticsearch/client/Response.java | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java index 89f456763bab4..42b2a6d7e25c9 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/Response.java +++ b/client/rest/src/main/java/org/elasticsearch/client/Response.java @@ -116,6 +116,29 @@ public HttpEntity getEntity() { "GMT" + // GMT "\")?"); // closing quote (optional, since an older version can still send a warn-date) + /** + * Optimized regular expression to test if a string matches the RFC 1123 date + * format (with quotes and leading space). Start/end of line characters and + * atomic groups are used to prevent backtracking. + */ + private static final Pattern WARNING_HEADER_DATE_PATTERN = Pattern.compile( + "^ " + // start of line, leading space + // quoted RFC 1123 date format + "\"" + // opening quote + "(?>Mon|Tue|Wed|Thu|Fri|Sat|Sun), " + // day of week, atomic group to prevent backtracking + "\\d{2} " + // 2-digit day + "(?>Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec) " + // month, atomic group to prevent backtracking + "\\d{4} " + // 4-digit year + "\\d{2}:\\d{2}:\\d{2} " + // (two-digit hour):(two-digit minute):(two-digit second) + "GMT" + // GMT + "\"$"); // closing quote (optional, since an older version can still send a warn-date), end of line + + /** + * Length of RFC 1123 format (with quotes and leading space), used in + * matchWarningHeaderPatternByPrefix(String). + */ + private static final int WARNING_HEADER_DATE_LENGTH = 32; + /** * Tests if a string matches the RFC 7234 specification for warning headers. * This assumes that the warn code is always 299 and the warn agent is always @@ -125,7 +148,24 @@ public HttpEntity getEntity() { * @return {@code true} if the input string matches the specification */ private static boolean matchWarningHeaderPatternByPrefix(final String s) { - return s.startsWith("299 Elasticsearch-"); + String warningHeader = s; + + /* + * The following block tests for the existence of a RFC 1123 date in the + * warning header. If the date exists, it is removed for + * extractWarningValueFromWarningHeader(String) to work properly (as it + * does not handle dates). + */ + if (s.length() > WARNING_HEADER_DATE_LENGTH) { + final String possibleDateString = s.substring(s.length() - WARNING_HEADER_DATE_LENGTH); + final Matcher matcher = WARNING_HEADER_DATE_PATTERN.matcher(possibleDateString); + + if (matcher.matches()) { + warningHeader = warningHeader.substring(0, s.length() - WARNING_HEADER_DATE_LENGTH); + } + } + + return warningHeader.startsWith("299 Elasticsearch-"); } /** From ae29214365afce112a30fcd4a7b242f925671481 Mon Sep 17 00:00:00 2001 From: Darren Foong Date: Mon, 16 Dec 2019 10:19:01 +0800 Subject: [PATCH 05/10] Fix previous commit; date removal should be done in extractWarningValueFromWarningHeader() --- .../org/elasticsearch/client/Response.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java index 42b2a6d7e25c9..a2819fa3305e7 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/Response.java +++ b/client/rest/src/main/java/org/elasticsearch/client/Response.java @@ -148,6 +148,13 @@ public HttpEntity getEntity() { * @return {@code true} if the input string matches the specification */ private static boolean matchWarningHeaderPatternByPrefix(final String s) { + return s.startsWith("299 Elasticsearch-"); + } + + /** + * Refer to org.elasticsearch.common.logging.DeprecationLogger + */ + private static String extractWarningValueFromWarningHeader(final String s) { String warningHeader = s; /* @@ -165,16 +172,9 @@ private static boolean matchWarningHeaderPatternByPrefix(final String s) { } } - return warningHeader.startsWith("299 Elasticsearch-"); - } - - /** - * Refer to org.elasticsearch.common.logging.DeprecationLogger - */ - private static String extractWarningValueFromWarningHeader(final String s) { - final int firstQuote = s.indexOf('\"'); - final int lastQuote = s.length() - 1; - final String warningValue = s.substring(firstQuote + 1, lastQuote); + final int firstQuote = warningHeader.indexOf('\"'); + final int lastQuote = warningHeader.length() - 1; + final String warningValue = warningHeader.substring(firstQuote + 1, lastQuote); assert assertWarningValue(s, warningValue); return warningValue; } From c7b15d9f33c51f6c9d29640b1135946e08e0ebe8 Mon Sep 17 00:00:00 2001 From: Darren Foong Date: Mon, 16 Dec 2019 10:27:55 +0800 Subject: [PATCH 06/10] Fix formatting --- .../main/java/org/elasticsearch/client/Response.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java index a2819fa3305e7..7979bf9e47fec 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/Response.java +++ b/client/rest/src/main/java/org/elasticsearch/client/Response.java @@ -164,12 +164,12 @@ private static String extractWarningValueFromWarningHeader(final String s) { * does not handle dates). */ if (s.length() > WARNING_HEADER_DATE_LENGTH) { - final String possibleDateString = s.substring(s.length() - WARNING_HEADER_DATE_LENGTH); - final Matcher matcher = WARNING_HEADER_DATE_PATTERN.matcher(possibleDateString); + final String possibleDateString = s.substring(s.length() - WARNING_HEADER_DATE_LENGTH); + final Matcher matcher = WARNING_HEADER_DATE_PATTERN.matcher(possibleDateString); - if (matcher.matches()) { - warningHeader = warningHeader.substring(0, s.length() - WARNING_HEADER_DATE_LENGTH); - } + if (matcher.matches()) { + warningHeader = warningHeader.substring(0, s.length() - WARNING_HEADER_DATE_LENGTH); + } } final int firstQuote = warningHeader.indexOf('\"'); From 564cb8a09227c8129b68cb44a65b189cd4ebfd05 Mon Sep 17 00:00:00 2001 From: Darren Foong Date: Mon, 16 Dec 2019 10:30:07 +0800 Subject: [PATCH 07/10] Remove final keyword --- .../rest/src/main/java/org/elasticsearch/client/Response.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java index 7979bf9e47fec..d5fbb0fd26f96 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/Response.java +++ b/client/rest/src/main/java/org/elasticsearch/client/Response.java @@ -195,7 +195,7 @@ private static boolean assertWarningValue(final String s, final String warningVa public List getWarnings() { List warnings = new ArrayList<>(); for (Header header : response.getHeaders("Warning")) { - final String warning = header.getValue(); + String warning = header.getValue(); if (matchWarningHeaderPatternByPrefix(warning)) { warnings.add(extractWarningValueFromWarningHeader(warning)); } else { From a493561920a47e88a1396939aae231da9ba62fff Mon Sep 17 00:00:00 2001 From: Darren Foong Date: Sun, 12 Jan 2020 17:46:05 +0800 Subject: [PATCH 08/10] Update WARNING_HEADER_DATE_LENGTH constant to reflect structure of WARNING_HEADER_DATE_PATTERN --- .../main/java/org/elasticsearch/client/Response.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java index d5fbb0fd26f96..abe96501a1a65 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/Response.java +++ b/client/rest/src/main/java/org/elasticsearch/client/Response.java @@ -137,7 +137,16 @@ public HttpEntity getEntity() { * Length of RFC 1123 format (with quotes and leading space), used in * matchWarningHeaderPatternByPrefix(String). */ - private static final int WARNING_HEADER_DATE_LENGTH = 32; + private static final int WARNING_HEADER_DATE_LENGTH = 0 + + 1 + + 1 + + 3 + 1 + 1 + + 2 + 1 + + 3 + 1 + + 4 + 1 + + 2 + 1 + 2 + 1 + 2 + 1 + + 3 + + 1; /** * Tests if a string matches the RFC 7234 specification for warning headers. From 2b096ea907d32066ca0f25fbb93574faec7c4d23 Mon Sep 17 00:00:00 2001 From: Darren Foong Date: Sun, 12 Jan 2020 17:55:26 +0800 Subject: [PATCH 09/10] Rewrite comment to use 140 column limit --- .../src/main/java/org/elasticsearch/client/Response.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java index abe96501a1a65..7abe24aa59eb5 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/Response.java +++ b/client/rest/src/main/java/org/elasticsearch/client/Response.java @@ -167,10 +167,8 @@ private static String extractWarningValueFromWarningHeader(final String s) { String warningHeader = s; /* - * The following block tests for the existence of a RFC 1123 date in the - * warning header. If the date exists, it is removed for - * extractWarningValueFromWarningHeader(String) to work properly (as it - * does not handle dates). + * The following block tests for the existence of a RFC 1123 date in the warning header. If the date exists, it is removed for + * extractWarningValueFromWarningHeader(String) to work properly (as it does not handle dates). */ if (s.length() > WARNING_HEADER_DATE_LENGTH) { final String possibleDateString = s.substring(s.length() - WARNING_HEADER_DATE_LENGTH); From 08b53ebe3d789a395424bc0e7fafaba9a5340feb Mon Sep 17 00:00:00 2001 From: Darren Foong Date: Sun, 12 Jan 2020 18:25:03 +0800 Subject: [PATCH 10/10] Update testDeprecationWarnings() to cover warnings without dates, and a sample warning from RFC 7234 --- .../client/RestClientSingleHostTests.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java index dd133f90daadb..fa5e9bcc6b43c 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java @@ -424,6 +424,7 @@ public void testHeaders() throws Exception { public void testDeprecationWarnings() throws Exception { String chars = randomAsciiAlphanumOfLength(5); assertDeprecationWarnings(singletonList("poorly formatted " + chars), singletonList("poorly formatted " + chars)); + assertDeprecationWarnings(singletonList(formatWarningWithoutDate(chars)), singletonList(chars)); assertDeprecationWarnings(singletonList(formatWarning(chars)), singletonList(chars)); assertDeprecationWarnings( Arrays.asList(formatWarning(chars), "another one", "and another"), @@ -433,6 +434,9 @@ public void testDeprecationWarnings() throws Exception { Arrays.asList("ignorable one", "and another")); assertDeprecationWarnings(singletonList("exact"), singletonList("exact")); assertDeprecationWarnings(Collections.emptyList(), Collections.emptyList()); + + String proxyWarning = "112 - \"network down\" \"Sat, 25 Aug 2012 23:34:45 GMT\""; + assertDeprecationWarnings(singletonList(proxyWarning), singletonList(proxyWarning)); } private enum DeprecationWarningOption { @@ -518,9 +522,13 @@ private void assertDeprecationWarnings(List warningHeaderTexts, List