From c8ed44d435097512b40ec6ccd91de49510c60b33 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 5 Jul 2021 15:41:57 +1000 Subject: [PATCH 1/3] Fix #6493 header cache memory usage Delay creating a header cache until a second request on a parser. Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/http/HttpParser.java | 40 +++++++++++++++---- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index 52844a5d9eb2..51d8b19770cb 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java @@ -15,6 +15,7 @@ import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.EnumSet; import java.util.LinkedHashMap; import java.util.List; @@ -246,6 +247,7 @@ public enum State private final StringBuilder _string = new StringBuilder(); private int _headerCacheSize = 1024; private boolean _headerCacheCaseSensitive; + private List _cacheableFields; private static HttpCompliance compliance() { @@ -746,6 +748,7 @@ private boolean parseLine(ByteBuffer buffer) break; case LF: + checkHeaderCache(); setState(State.HEADER); _responseHandler.startResponse(_version, _responseStatus, null); break; @@ -851,6 +854,7 @@ else if (n == HttpTokens.LINE_FEED) case LF: if (_responseHandler != null) { + checkHeaderCache(); setState(State.HEADER); _responseHandler.startResponse(_version, _responseStatus, null); } @@ -881,7 +885,7 @@ else if (n == HttpTokens.LINE_FEED) _version = HttpVersion.CACHE.get(takeString()); } checkVersion(); - + checkHeaderCache(); setState(State.HEADER); _requestHandler.startRequest(_methodString, _uri.toString(), _version); @@ -905,6 +909,7 @@ else if (n == HttpTokens.LINE_FEED) { case LF: String reason = takeString(); + checkHeaderCache(); setState(State.HEADER); _responseHandler.startResponse(_version, _responseStatus, reason); continue; @@ -937,6 +942,21 @@ else if (n == HttpTokens.LINE_FEED) return handle; } + private void checkHeaderCache() + { + if (_fieldCache == null && _cacheableFields != null) + { + _fieldCache = Index.buildCaseSensitiveMutableVisibleAsciiAlphabet(getHeaderCacheSize()); + for (HttpField f : _cacheableFields) + { + if (!_fieldCache.put(f)) + break; + } + _cacheableFields.clear(); + _cacheableFields = null; + } + } + private void checkVersion() { if (_version == null) @@ -1026,7 +1046,7 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT) _field = new HostPortHttpField(_header, CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode) ? _headerString : _header.asString(), _valueString); - addToFieldCache = true; + addToFieldCache = getHeaderCacheSize() > 0; } break; @@ -1046,7 +1066,7 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT) case COOKIE: case CACHE_CONTROL: case USER_AGENT: - addToFieldCache = _field == null; + addToFieldCache = _field == null && getHeaderCacheSize() > 0 && _valueString.length() < getHeaderCacheSize(); break; default: @@ -1054,14 +1074,18 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT) } // Cache field? - if (addToFieldCache && _header != null && _valueString != null) + if (addToFieldCache && _fieldCache != NO_CACHE && _header != null && _valueString != null) { - if (_fieldCache == null) - _fieldCache = Index.buildCaseSensitiveMutableVisibleAsciiAlphabet(getHeaderCacheSize()); - if (_field == null) _field = new HttpField(_header, caseInsensitiveHeader(_headerString, _header.asString()), _valueString); - if (_field.getValue().length() < getHeaderCacheSize() && !_fieldCache.put(_field)) + + if (_fieldCache == null) + { + if (_cacheableFields == null) + _cacheableFields = new ArrayList<>(); + _cacheableFields.add(_field); + } + else if (!_fieldCache.put(_field)) { _fieldCache.clear(); _fieldCache.put(_field); From 4dca32c6a3fbcd5dc079e231a4a3f10dd90f4111 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 5 Jul 2021 16:22:22 +1000 Subject: [PATCH 2/3] Fix #6493 header cache memory usage Delay creating a header cache until a second request on a parser. Signed-off-by: Greg Wilkins --- jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java | 1 + 1 file changed, 1 insertion(+) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index 51d8b19770cb..41c7117a460d 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java @@ -1935,6 +1935,7 @@ protected void setState(FieldState state) public Index getFieldCache() { + checkHeaderCache(); return _fieldCache; } From 34967fbe249e41dcbd7c9045d8d37a15f1db01d6 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 6 Jul 2021 14:47:46 +1000 Subject: [PATCH 3/3] Fix #6493 header cache memory usage Refactored cache code into subclass Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/http/HttpParser.java | 145 ++++++++++++------ .../java/org/eclipse/jetty/util/Index.java | 8 +- 2 files changed, 104 insertions(+), 49 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index 41c7117a460d..dad562a72e41 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java @@ -218,6 +218,7 @@ public enum State private final int _maxHeaderBytes; private final HttpCompliance _complianceMode; private final Utf8StringBuilder _uri = new Utf8StringBuilder(INITIAL_URI_LENGTH); + private final FieldCache _fieldCache = new FieldCache(); private HttpField _field; private HttpHeader _header; private String _headerString; @@ -242,12 +243,8 @@ public enum State private boolean _headResponse; private boolean _cr; private ByteBuffer _contentChunk; - private Index.Mutable _fieldCache; private int _length; private final StringBuilder _string = new StringBuilder(); - private int _headerCacheSize = 1024; - private boolean _headerCacheCaseSensitive; - private List _cacheableFields; private static HttpCompliance compliance() { @@ -306,22 +303,22 @@ public HttpHandler getHandler() public int getHeaderCacheSize() { - return _headerCacheSize; + return _fieldCache.getCapacity(); } public void setHeaderCacheSize(int headerCacheSize) { - _headerCacheSize = headerCacheSize; + _fieldCache.setCapacity(headerCacheSize); } public boolean isHeaderCacheCaseSensitive() { - return _headerCacheCaseSensitive; + return _fieldCache.isCaseSensitive(); } public void setHeaderCacheCaseSensitive(boolean headerCacheCaseSensitive) { - _headerCacheCaseSensitive = headerCacheCaseSensitive; + _fieldCache.setCaseSensitive(headerCacheCaseSensitive); } protected void checkViolation(Violation violation) throws BadMessageException @@ -748,7 +745,7 @@ private boolean parseLine(ByteBuffer buffer) break; case LF: - checkHeaderCache(); + _fieldCache.prepare(); setState(State.HEADER); _responseHandler.startResponse(_version, _responseStatus, null); break; @@ -854,7 +851,7 @@ else if (n == HttpTokens.LINE_FEED) case LF: if (_responseHandler != null) { - checkHeaderCache(); + _fieldCache.prepare(); setState(State.HEADER); _responseHandler.startResponse(_version, _responseStatus, null); } @@ -885,7 +882,7 @@ else if (n == HttpTokens.LINE_FEED) _version = HttpVersion.CACHE.get(takeString()); } checkVersion(); - checkHeaderCache(); + _fieldCache.prepare(); setState(State.HEADER); _requestHandler.startRequest(_methodString, _uri.toString(), _version); @@ -909,7 +906,7 @@ else if (n == HttpTokens.LINE_FEED) { case LF: String reason = takeString(); - checkHeaderCache(); + _fieldCache.prepare(); setState(State.HEADER); _responseHandler.startResponse(_version, _responseStatus, reason); continue; @@ -942,21 +939,6 @@ else if (n == HttpTokens.LINE_FEED) return handle; } - private void checkHeaderCache() - { - if (_fieldCache == null && _cacheableFields != null) - { - _fieldCache = Index.buildCaseSensitiveMutableVisibleAsciiAlphabet(getHeaderCacheSize()); - for (HttpField f : _cacheableFields) - { - if (!_fieldCache.put(f)) - break; - } - _cacheableFields.clear(); - _cacheableFields = null; - } - } - private void checkVersion() { if (_version == null) @@ -1046,7 +1028,7 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT) _field = new HostPortHttpField(_header, CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode) ? _headerString : _header.asString(), _valueString); - addToFieldCache = getHeaderCacheSize() > 0; + addToFieldCache = _fieldCache.isEnabled(); } break; @@ -1055,7 +1037,7 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT) if (_field == null) _field = new HttpField(_header, caseInsensitiveHeader(_headerString, _header.asString()), _valueString); if (getHeaderCacheSize() > 0 && _field.contains(HttpHeaderValue.CLOSE.asString())) - _fieldCache = NO_CACHE; + _fieldCache.setCapacity(-1); break; case AUTHORIZATION: @@ -1066,7 +1048,7 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT) case COOKIE: case CACHE_CONTROL: case USER_AGENT: - addToFieldCache = _field == null && getHeaderCacheSize() > 0 && _valueString.length() < getHeaderCacheSize(); + addToFieldCache = _field == null && _fieldCache.cacheable(_header, _valueString); break; default: @@ -1074,22 +1056,12 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT) } // Cache field? - if (addToFieldCache && _fieldCache != NO_CACHE && _header != null && _valueString != null) + if (addToFieldCache) { if (_field == null) _field = new HttpField(_header, caseInsensitiveHeader(_headerString, _header.asString()), _valueString); - if (_fieldCache == null) - { - if (_cacheableFields == null) - _cacheableFields = new ArrayList<>(); - _cacheableFields.add(_field); - } - else if (!_fieldCache.put(_field)) - { - _fieldCache.clear(); - _fieldCache.put(_field); - } + _fieldCache.add(_field); } } _handler.parsedHeader(_field != null ? _field : new HttpField(_header, _headerString, _valueString)); @@ -1268,7 +1240,7 @@ else if (_endOfContent == EndOfContent.UNKNOWN_CONTENT) if (buffer.hasRemaining()) { // Try a look ahead for the known header name and value. - HttpField cachedField = _fieldCache == null ? null : _fieldCache.getBest(buffer, -1, buffer.remaining()); + HttpField cachedField = _fieldCache.getBest(buffer, -1, buffer.remaining()); if (cachedField == null) cachedField = CACHE.getBest(buffer, -1, buffer.remaining()); @@ -1935,8 +1907,8 @@ protected void setState(FieldState state) public Index getFieldCache() { - checkHeaderCache(); - return _fieldCache; + _fieldCache.prepare(); + return _fieldCache.getCache(); } @Override @@ -2032,4 +2004,87 @@ private IllegalCharacterException(State state, HttpTokens.Token token, ByteBuffe LOG.debug(String.format("Illegal character %s in state=%s for buffer %s", token, state, BufferUtil.toDetailString(buffer))); } } + + private static class FieldCache + { + private int _size = 1024; + private Index.Mutable _cache; + private List _cacheableFields; + private boolean _caseSensitive; + + public int getCapacity() + { + return _size; + } + + public void setCapacity(int size) + { + _size = size; + if (_size <= 0) + _cache = NO_CACHE; + else + _cache = null; + } + + public boolean isCaseSensitive() + { + return _caseSensitive; + } + + public void setCaseSensitive(boolean caseSensitive) + { + _caseSensitive = caseSensitive; + } + + public boolean isEnabled() + { + return _cache != NO_CACHE; + } + + public Index getCache() + { + return _cache; + } + + public HttpField getBest(ByteBuffer buffer, int i, int remaining) + { + Index.Mutable cache = _cache; + return cache == null ? null : _cache.getBest(buffer, i, remaining); + } + + public void add(HttpField field) + { + if (_cache == null) + { + if (_cacheableFields == null) + _cacheableFields = new ArrayList<>(); + _cacheableFields.add(field); + } + else if (!_cache.put(field)) + { + _cache.clear(); + _cache.put(field); + } + } + + public boolean cacheable(HttpHeader header, String valueString) + { + return isEnabled() && header != null && valueString.length() <= _size; + } + + private void prepare() + { + if (_cache == null && _cacheableFields != null) + { + _cache = Index.buildMutableVisibleAsciiAlphabet(_caseSensitive, _size); + for (HttpField f : _cacheableFields) + { + if (!_cache.put(f)) + break; + } + _cacheableFields.clear(); + _cacheableFields = null; + } + } + } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Index.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Index.java index 1b79b0fa21c5..2e4e0656dacd 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/Index.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Index.java @@ -259,13 +259,13 @@ public Mutable build() * @param The type of the index * @return A case sensitive mutable Index tacking visible ASCII alphabet to a max capacity. */ - static Mutable buildCaseSensitiveMutableVisibleAsciiAlphabet(int maxCapacity) + static Mutable buildMutableVisibleAsciiAlphabet(boolean caseSensitive, int maxCapacity) { if (maxCapacity < 0 || maxCapacity > ArrayTrie.MAX_CAPACITY) - return new TreeTrie<>(true); + return new TreeTrie<>(caseSensitive); if (maxCapacity == 0) - return EmptyTrie.instance(true); - return new ArrayTrie<>(true, maxCapacity); + return EmptyTrie.instance(caseSensitive); + return new ArrayTrie<>(caseSensitive, maxCapacity); } static Index empty(boolean caseSensitive)