Remove warn-date from warning headers (#37622)

jasontedor · web-flow · commit 715719ee3b5e · 2019-01-22T12:29:24.000-05:00
This commit removes the warn-date from warning headers. Previously we
were stamping every warning header with when the request
occurred. However, this has a severe performance penalty when
deprecation logging is called frequently, as obtaining the current time
and formatting it properly is expensive. A previous change moved to
using the startup time as the time to stamp on every warning header, but
this was only to prove that the timestamping was expensive. Since the
warn-date is optional, we elect to remove it from the warning
header. Prior to this commit, we worked in Kibana to make the warn-date
treated as optional there so that we can follow-up in Elasticsearch and
remove the warn-date. This commit does that.
diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java
@@ -103,7 +103,7 @@ public HttpEntity getEntity() {
     private static final Pattern WARNING_HEADER_PATTERN = Pattern.compile(
             "299 " + // warn code
             "Elasticsearch-\\d+\\.\\d+\\.\\d+(?:-(?:alpha|beta|rc)\\d+)?(?:-SNAPSHOT)?-(?:[a-f0-9]{7}|Unknown) " + // warn agent
-            "\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\" " + // quoted warning value, captured
+            "\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\"( " + // quoted warning value, captured
             // quoted RFC 1123 date format
             "\"" + // opening quote
             "(?:Mon|Tue|Wed|Thu|Fri|Sat|Sun), " + // weekday
@@ -112,7 +112,7 @@ public HttpEntity getEntity() {
             "\\d{4} " + // 4-digit year
             "\\d{2}:\\d{2}:\\d{2} " + // (two-digit hour):(two-digit minute):(two-digit second)
             "GMT" + // GMT
-            "\""); // closing quote
+            "\")?"); // closing quote (optional, since an older version can still send a warn-date)
 
     /**
      * Returns a list of all warning headers returned in the response.
diff --git a/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java b/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java
@@ -29,14 +29,8 @@
 import java.nio.charset.Charset;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
-import java.time.ZoneId;
-import java.time.ZonedDateTime;
-import java.time.format.DateTimeFormatter;
-import java.time.format.DateTimeFormatterBuilder;
-import java.time.format.SignStyle;
 import java.util.BitSet;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Locale;
@@ -47,14 +41,6 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import static java.time.temporal.ChronoField.DAY_OF_MONTH;
-import static java.time.temporal.ChronoField.DAY_OF_WEEK;
-import static java.time.temporal.ChronoField.HOUR_OF_DAY;
-import static java.time.temporal.ChronoField.MINUTE_OF_HOUR;
-import static java.time.temporal.ChronoField.MONTH_OF_YEAR;
-import static java.time.temporal.ChronoField.SECOND_OF_MINUTE;
-import static java.time.temporal.ChronoField.YEAR;
-
 /**
  * A logger that logs deprecation notices.
  */
@@ -165,73 +151,14 @@ public void deprecatedAndMaybeLog(final String key, final String msg, final Obje
                     Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : "",
                     Build.CURRENT.shortHash());
 
-    /*
-     * RFC 7234 section 5.5 specifies that the warn-date is a quoted HTTP-date. HTTP-date is defined in RFC 7234 Appendix B as being from
-     * RFC 7231 section 7.1.1.1. RFC 7231 specifies an HTTP-date as an IMF-fixdate (or an obs-date referring to obsolete formats). The
-     * grammar for IMF-fixdate is specified as 'day-name "," SP date1 SP time-of-day SP GMT'. Here, day-name is
-     * (Mon|Tue|Wed|Thu|Fri|Sat|Sun). Then, date1 is 'day SP month SP year' where day is 2DIGIT, month is
-     * (Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec), and year is 4DIGIT. Lastly, time-of-day is 'hour ":" minute ":" second' where
-     * hour is 2DIGIT, minute is 2DIGIT, and second is 2DIGIT. Finally, 2DIGIT and 4DIGIT have the obvious definitions.
-     */
-    private static final DateTimeFormatter RFC_7231_DATE_TIME;
-
-    static {
-        final Map<Long, String> dow = new HashMap<>();
-        dow.put(1L, "Mon");
-        dow.put(2L, "Tue");
-        dow.put(3L, "Wed");
-        dow.put(4L, "Thu");
-        dow.put(5L, "Fri");
-        dow.put(6L, "Sat");
-        dow.put(7L, "Sun");
-        final Map<Long, String> moy = new HashMap<>();
-        moy.put(1L, "Jan");
-        moy.put(2L, "Feb");
-        moy.put(3L, "Mar");
-        moy.put(4L, "Apr");
-        moy.put(5L, "May");
-        moy.put(6L, "Jun");
-        moy.put(7L, "Jul");
-        moy.put(8L, "Aug");
-        moy.put(9L, "Sep");
-        moy.put(10L, "Oct");
-        moy.put(11L, "Nov");
-        moy.put(12L, "Dec");
-        RFC_7231_DATE_TIME = new DateTimeFormatterBuilder()
-                .parseCaseInsensitive()
-                .parseLenient()
-                .optionalStart()
-                .appendText(DAY_OF_WEEK, dow)
-                .appendLiteral(", ")
-                .optionalEnd()
-                .appendValue(DAY_OF_MONTH, 2, 2, SignStyle.NOT_NEGATIVE)
-                .appendLiteral(' ')
-                .appendText(MONTH_OF_YEAR, moy)
-                .appendLiteral(' ')
-                .appendValue(YEAR, 4)
-                .appendLiteral(' ')
-                .appendValue(HOUR_OF_DAY, 2)
-                .appendLiteral(':')
-                .appendValue(MINUTE_OF_HOUR, 2)
-                .optionalStart()
-                .appendLiteral(':')
-                .appendValue(SECOND_OF_MINUTE, 2)
-                .optionalEnd()
-                .appendLiteral(' ')
-                .appendOffset("+HHMM", "GMT")
-                .toFormatter(Locale.getDefault(Locale.Category.FORMAT));
-    }
-
-    private static final String STARTUP_TIME = RFC_7231_DATE_TIME.format(ZonedDateTime.now(ZoneId.of("GMT")));
-
     /**
      * Regular expression to test if a string matches the RFC7234 specification for warning headers. This pattern assumes that the warn code
      * is always 299. Further, this pattern assumes that the warn agent represents a version of Elasticsearch including the build hash.
      */
-    public static Pattern WARNING_HEADER_PATTERN = Pattern.compile(
+    public static final Pattern WARNING_HEADER_PATTERN = Pattern.compile(
             "299 " + // warn code
                     "Elasticsearch-\\d+\\.\\d+\\.\\d+(?:-(?:alpha|beta|rc)\\d+)?(?:-SNAPSHOT)?-(?:[a-f0-9]{7}|Unknown) " + // warn agent
-                    "\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\" " + // quoted warning value, captured
+                    "\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\"( " + // quoted warning value, captured
                     // quoted RFC 1123 date format
                     "\"" + // opening quote
                     "(?:Mon|Tue|Wed|Thu|Fri|Sat|Sun), " + // weekday
@@ -240,36 +167,31 @@ public void deprecatedAndMaybeLog(final String key, final String msg, final Obje
                     "\\d{4} " + // 4-digit year
                     "\\d{2}:\\d{2}:\\d{2} " + // (two-digit hour):(two-digit minute):(two-digit second)
                     "GMT" + // GMT
-                    "\""); // closing quote
+                    "\")?"); // closing quote (optional, since an older version can still send a warn-date)
 
     /**
      * Extracts the warning value from the value of a warning header that is formatted according to RFC 7234. That is, given a string
-     * {@code 299 Elasticsearch-6.0.0 "warning value" "Sat, 25 Feb 2017 10:27:43 GMT"}, the return value of this method would be {@code
-     * warning value}.
+     * {@code 299 Elasticsearch-6.0.0 "warning value"}, the return value of this method would be {@code warning value}.
      *
      * @param s the value of a warning header formatted according to RFC 7234.
      * @return the extracted warning value
      */
     public static String extractWarningValueFromWarningHeader(final String s) {
         /*
          * We know the exact format of the warning header, so to extract the warning value we can skip forward from the front to the first
-         * quote, and skip backwards from the end to the penultimate quote:
+         * quote and we know the last quote is at the end of the string
          *
-         *   299 Elasticsearch-6.0.0 "warning value" "Sat, 25, Feb 2017 10:27:43 GMT"
-         *                           ^               ^                              ^
-         *                           firstQuote      penultimateQuote               lastQuote
-         *
-         * We do it this way rather than seeking forward after the first quote because there could be escaped quotes in the warning value
-         * but since there are none in the warning date, we can skip backwards to find the quote that closes the quoted warning value.
+         *   299 Elasticsearch-6.0.0 "warning value"
+         *                           ^             ^
+         *                           firstQuote    lastQuote
          *
          * We parse this manually rather than using the capturing regular expression because the regular expression involves a lot of
          * backtracking and carries a performance penalty. However, when assertions are enabled, we still use the regular expression to
          * verify that we are maintaining the warning header format.
          */
         final int firstQuote = s.indexOf('\"');
-        final int lastQuote = s.lastIndexOf('\"');
-        final int penultimateQuote = s.lastIndexOf('\"', lastQuote - 1);
-        final String warningValue = s.substring(firstQuote + 1, penultimateQuote - 2);
+        final int lastQuote = s.length() - 1;
+        final String warningValue = s.substring(firstQuote + 1, lastQuote);
         assert assertWarningValue(s, warningValue);
         return warningValue;
     }
@@ -299,7 +221,6 @@ void deprecated(final Set<ThreadContext> threadContexts, final String message, f
         deprecated(threadContexts, message, true, params);
     }
 
-
     void deprecated(final Set<ThreadContext> threadContexts, final String message, final boolean log, final Object... params) {
         final Iterator<ThreadContext> iterator = threadContexts.iterator();
 
@@ -338,9 +259,7 @@ public Void run() {
      * @return a warning value formatted according to RFC 7234
      */
     public static String formatWarning(final String s) {
-        return WARNING_PREFIX + " "
-                + "\"" + escapeAndEncode(s) + "\"" + " "
-                + "\"" + STARTUP_TIME + "\"";
+        return WARNING_PREFIX + " " + "\"" + escapeAndEncode(s) + "\"";
     }
 
     /**
@@ -451,7 +370,7 @@ static String encode(final String s) {
                 int startIndex = i;
                 do {
                     i++;
-                } while (i < s.length() && !doesNotNeedEncoding.get(s.charAt(i)));
+                } while (i < s.length() && doesNotNeedEncoding.get(s.charAt(i)) == false);
 
                 final byte[] bytes = s.substring(startIndex, i).getBytes(UTF_8);
                 // noinspection ForLoopReplaceableByForEach
