From c5df807b6d5de75118aca797dc168327b7364ebd Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 13 Oct 2020 23:18:26 +0200 Subject: [PATCH 1/3] Fixes #5409 - HttpClient fails intermittently with "Invalid response state TRANSIENT". The problem was a race condition during content decoding. Since decoding needs to be done in a loop, the condition to loop is to check whether there is demand for the next chunk of decoded content. Checking for demand also sets the stalled flag, and this must be done only after the response state has been set back to CONTENT. Unfortunately this was not done in the decoding loop. The fix is to always update the response state in the decoding loop. Signed-off-by: Simone Bordet --- .../eclipse/jetty/client/HttpExchange.java | 6 +- .../eclipse/jetty/client/HttpReceiver.java | 269 +++++++++--------- .../jetty/client/HttpClientGZIPTest.java | 45 +++ 3 files changed, 184 insertions(+), 136 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java index 94d204fe559..f90feaa1215 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java @@ -290,11 +290,11 @@ public class HttpExchange { synchronized (this) { - return String.format("%s@%x req=%s/%s@%h res=%s/%s@%h", + return String.format("%s@%x{req=%s[%s/%s] res=%s[%s/%s]}", HttpExchange.class.getSimpleName(), hashCode(), - requestState, requestFailure, requestFailure, - responseState, responseFailure, responseFailure); + request, requestState, requestFailure, + response, responseState, responseFailure); } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java index 27127388768..35df5596fdc 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java @@ -187,7 +187,7 @@ public abstract class HttpReceiver { handlerListener = protocolHandler.getResponseListener(); if (LOG.isDebugEnabled()) - LOG.debug("Found protocol handler {}", protocolHandler); + LOG.debug("Response {} found protocol handler {}", response, protocolHandler); } exchange.getConversation().updateResponseListeners(handlerListener); @@ -218,19 +218,8 @@ public abstract class HttpReceiver */ protected boolean responseHeader(HttpExchange exchange, HttpField field) { - while (true) - { - ResponseState current = responseState.get(); - if (current == ResponseState.BEGIN || current == ResponseState.HEADER) - { - if (updateResponseState(current, ResponseState.TRANSIENT)) - break; - } - else - { - return false; - } - } + if (!updateResponseState(ResponseState.BEGIN, ResponseState.HEADER, ResponseState.TRANSIENT)) + return false; HttpResponse response = exchange.getResponse(); ResponseNotifier notifier = getHttpDestination().getResponseNotifier(); @@ -296,19 +285,8 @@ public abstract class HttpReceiver */ protected boolean responseHeaders(HttpExchange exchange) { - while (true) - { - ResponseState current = responseState.get(); - if (current == ResponseState.BEGIN || current == ResponseState.HEADER) - { - if (updateResponseState(current, ResponseState.TRANSIENT)) - break; - } - else - { - return false; - } - } + if (!updateResponseState(ResponseState.BEGIN, ResponseState.HEADER, ResponseState.TRANSIENT)) + return false; HttpResponse response = exchange.getResponse(); if (LOG.isDebugEnabled()) @@ -342,7 +320,7 @@ public abstract class HttpReceiver { boolean hasDemand = hasDemandOrStall(); if (LOG.isDebugEnabled()) - LOG.debug("Response headers {}, hasDemand={}", response, hasDemand); + LOG.debug("Response headers hasDemand={} {}", hasDemand, response); return hasDemand; } @@ -363,71 +341,39 @@ public abstract class HttpReceiver */ protected boolean responseContent(HttpExchange exchange, ByteBuffer buffer, Callback callback) { - while (true) - { - ResponseState current = responseState.get(); - if (current == ResponseState.HEADERS || current == ResponseState.CONTENT) - { - if (updateResponseState(current, ResponseState.TRANSIENT)) - break; - } - else - { - callback.failed(new IllegalStateException("Invalid response state " + current)); - return false; - } - } - - boolean proceed = true; + if (LOG.isDebugEnabled()) + LOG.debug("Response content {}{}{}", exchange.getResponse(), System.lineSeparator(), BufferUtil.toDetailString(buffer)); if (demand() <= 0) { callback.failed(new IllegalStateException("No demand for response content")); - proceed = false; + return false; + } + if (decoder == null) + return plainResponseContent(exchange, buffer, callback); + else + return decodeResponseContent(buffer, callback); + } + + private boolean plainResponseContent(HttpExchange exchange, ByteBuffer buffer, Callback callback) + { + if (!updateResponseState(ResponseState.HEADERS, ResponseState.CONTENT, ResponseState.TRANSIENT)) + { + callback.failed(new IllegalStateException("Invalid response state " + responseState)); + return false; } HttpResponse response = exchange.getResponse(); - if (proceed) - { - if (LOG.isDebugEnabled()) - LOG.debug("Response content {}{}{}", response, System.lineSeparator(), BufferUtil.toDetailString(buffer)); - if (contentListeners.isEmpty()) - { - callback.succeeded(); - } - else - { - if (decoder == null) - { - contentListeners.notifyContent(response, buffer, callback); - } - else - { - try - { - proceed = decoder.decode(buffer, callback); - } - catch (Throwable x) - { - callback.failed(x); - proceed = false; - } - } - } - } + if (contentListeners.isEmpty()) + callback.succeeded(); + else + contentListeners.notifyContent(response, buffer, callback); if (updateResponseState(ResponseState.TRANSIENT, ResponseState.CONTENT)) { - if (proceed) - { - boolean hasDemand = hasDemandOrStall(); - if (LOG.isDebugEnabled()) - LOG.debug("Response content {}, hasDemand={}", response, hasDemand); - return hasDemand; - } - else - { - return false; - } + boolean hasDemand = hasDemandOrStall(); + if (LOG.isDebugEnabled()) + LOG.debug("Response content {}, hasDemand={}", response, hasDemand); + return hasDemand; } dispose(); @@ -435,6 +381,11 @@ public abstract class HttpReceiver return false; } + private boolean decodeResponseContent(ByteBuffer buffer, Callback callback) + { + return decoder.decode(buffer, callback); + } + /** * Method to be invoked when the response is successful. *

@@ -614,15 +565,42 @@ public abstract class HttpReceiver } } + private boolean updateResponseState(ResponseState from1, ResponseState from2, ResponseState to) + { + while (true) + { + ResponseState current = responseState.get(); + if (current == from1 || current == from2) + { + if (updateResponseState(current, to)) + return true; + } + else + { + if (LOG.isDebugEnabled()) + LOG.debug("State update failed: [{},{}] -> {}: {}", from1, from2, to, current); + return false; + } + } + } + private boolean updateResponseState(ResponseState from, ResponseState to) { - boolean updated = responseState.compareAndSet(from, to); - if (!updated) + while (true) { - if (LOG.isDebugEnabled()) - LOG.debug("State update failed: {} -> {}: {}", from, to, responseState.get()); + ResponseState current = responseState.get(); + if (current == from) + { + if (responseState.compareAndSet(current, to)) + return true; + } + else + { + if (LOG.isDebugEnabled()) + LOG.debug("State update failed: {} -> {}: {}", from, to, current); + return false; + } } - return updated; } @Override @@ -778,14 +756,62 @@ public abstract class HttpReceiver private boolean decode(ByteBuffer encoded, Callback callback) { + // Store the buffer to decode in case the + // decoding produces multiple decoded buffers. this.encoded = encoded; this.callback = callback; - return decode(); + + HttpResponse response = exchange.getResponse(); + if (LOG.isDebugEnabled()) + LOG.debug("Response content decoding {} with {}{}{}", response, decoder, System.lineSeparator(), BufferUtil.toDetailString(encoded)); + + boolean needInput = decode(); + if (!needInput) + return false; + + boolean hasDemand = hasDemandOrStall(); + if (LOG.isDebugEnabled()) + LOG.debug("Response content decoded, hasDemand={} {}", hasDemand, response); + return hasDemand; } private boolean decode() { while (true) + { + if (!updateResponseState(ResponseState.HEADERS, ResponseState.CONTENT, ResponseState.TRANSIENT)) + { + callback.failed(new IllegalStateException("Invalid response state " + responseState)); + return false; + } + + DecodeResult result = decodeChunk(); + + if (updateResponseState(ResponseState.TRANSIENT, ResponseState.CONTENT)) + { + if (result == DecodeResult.NEED_INPUT) + return true; + if (result == DecodeResult.ABORT) + return false; + + boolean hasDemand = hasDemandOrStall(); + if (LOG.isDebugEnabled()) + LOG.debug("Response content decoded chunk, hasDemand={} {}", hasDemand, exchange.getResponse()); + if (hasDemand) + continue; + else + return false; + } + + dispose(); + terminateResponse(exchange); + return false; + } + } + + private DecodeResult decodeChunk() + { + try { ByteBuffer buffer; while (true) @@ -798,27 +824,30 @@ public abstract class HttpReceiver callback.succeeded(); encoded = null; callback = null; - return true; + return DecodeResult.NEED_INPUT; } } + ByteBuffer decoded = buffer; + HttpResponse response = exchange.getResponse(); if (LOG.isDebugEnabled()) - LOG.debug("Response content decoded ({}) {}{}{}", decoder, exchange, System.lineSeparator(), BufferUtil.toDetailString(decoded)); + LOG.debug("Response content decoded chunk {}{}{}", response, System.lineSeparator(), BufferUtil.toDetailString(decoded)); - contentListeners.notifyContent(exchange.getResponse(), decoded, Callback.from(() -> decoder.release(decoded), callback::failed)); + contentListeners.notifyContent(response, decoded, Callback.from(() -> decoder.release(decoded), callback::failed)); - boolean hasDemand = hasDemandOrStall(); - if (LOG.isDebugEnabled()) - LOG.debug("Response content decoded {}, hasDemand={}", exchange, hasDemand); - if (!hasDemand) - return false; + return DecodeResult.DECODE; + } + catch (Throwable x) + { + callback.failed(x); + return DecodeResult.ABORT; } } private void resume() { if (LOG.isDebugEnabled()) - LOG.debug("Response content resuming decoding {}", exchange); + LOG.debug("Response content resume decoding {} with {}", exchange.getResponse(), decoder); // The content and callback may be null // if there is no initial content demand. @@ -828,40 +857,9 @@ public abstract class HttpReceiver return; } - while (true) - { - ResponseState current = responseState.get(); - if (current == ResponseState.HEADERS || current == ResponseState.CONTENT) - { - if (updateResponseState(current, ResponseState.TRANSIENT)) - break; - } - else - { - callback.failed(new IllegalStateException("Invalid response state " + current)); - return; - } - } - - boolean decoded = false; - try - { - decoded = decode(); - } - catch (Throwable x) - { - callback.failed(x); - } - - if (updateResponseState(ResponseState.TRANSIENT, ResponseState.CONTENT)) - { - if (decoded) - receive(); - return; - } - - dispose(); - terminateResponse(exchange); + boolean needInput = decode(); + if (needInput) + receive(); } @Override @@ -871,4 +869,9 @@ public abstract class HttpReceiver ((Destroyable)decoder).destroy(); } } + + private enum DecodeResult + { + DECODE, NEED_INPUT, ABORT + } } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java index e29f0a3b573..3120a97619d 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java @@ -20,6 +20,7 @@ package org.eclipse.jetty.client; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.InterruptedIOException; import java.nio.charset.StandardCharsets; import java.util.Arrays; @@ -32,11 +33,14 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.client.api.Response; +import org.eclipse.jetty.client.util.InputStreamResponseListener; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.MappedByteBufferPool; import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.util.IO; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; @@ -266,6 +270,47 @@ public class HttpClientGZIPTest extends AbstractHttpClientServerTest assertThat(heapMemory, lessThan((long)content.length)); } + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testLargeGZIPContentAsync(Scenario scenario) throws Exception + { + String digits = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; + Random random = new Random(); + byte[] content = new byte[32 * 1024 * 1024]; + for (int i = 0; i < content.length; ++i) + { + content[i] = (byte)digits.charAt(random.nextInt(digits.length())); + } + start(scenario, new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.setContentType("text/plain;charset=" + StandardCharsets.US_ASCII.name()); + response.setHeader(HttpHeader.CONTENT_ENCODING.asString(), "gzip"); + GZIPOutputStream gzip = new GZIPOutputStream(response.getOutputStream()); + gzip.write(content); + gzip.finish(); + } + }); + + InputStreamResponseListener listener = new InputStreamResponseListener(); + client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .timeout(5, TimeUnit.SECONDS) + .send(listener); + + Response response = listener.get(5, TimeUnit.SECONDS); + assertEquals(HttpStatus.OK_200, response.getStatus()); + + ByteArrayOutputStream output = new ByteArrayOutputStream(); + try (InputStream input = listener.getInputStream()) + { + IO.copy(input, output); + } + assertArrayEquals(content, output.toByteArray()); + } + private static void sleep(long ms) throws IOException { try From d78e1f8a30bb0d9676f659432a667b70abbf23ee Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Wed, 14 Oct 2020 13:03:28 +0200 Subject: [PATCH 2/3] Issue #5444 Fix deploy-jndi.adoc Signed-off-by: Jan Bartel --- .../operations-guide/deploy/deploy-jndi.adoc | 13 ++++++++----- .../asciidoc/operations-guide/jndi/chapter.adoc | 7 +++---- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/jetty-documentation/src/main/asciidoc/operations-guide/deploy/deploy-jndi.adoc b/jetty-documentation/src/main/asciidoc/operations-guide/deploy/deploy-jndi.adoc index 904eaecc172..8f412e4ece6 100644 --- a/jetty-documentation/src/main/asciidoc/operations-guide/deploy/deploy-jndi.adoc +++ b/jetty-documentation/src/main/asciidoc/operations-guide/deploy/deploy-jndi.adoc @@ -20,7 +20,7 @@ ==== Configuring JNDI Entries A web application may _reference_ a JNDI entry, such as a JDBC `DataSource` from the web application `web.xml` file. -The JNDI entry must be _defined_ in the Jetty context XML file, for example: +The JNDI entry must be _defined_ in a link:#og-jndi-xml[Jetty XML file], for example a context XML like so: .mywebapp.xml [source,xml,subs=normal] @@ -28,11 +28,11 @@ The JNDI entry must be _defined_ in the Jetty context XML file, for example: - + /mywebapp /opt/webapps/mywebapp.war #   - + jdbc/myds @@ -45,11 +45,14 @@ The JNDI entry must be _defined_ in the Jetty context XML file, for example: ---- +For more information and examples on how to use JNDI in Jetty, refer to the link:#og-jndi[JNDI] feature section. + [IMPORTANT] ==== -Class `com.mysql.cj.jdbc.MysqlConnectionPoolDataSource` is present in the MySQL JDBC driver file, `mysql-connector-java-.jar`, which must be available to the web application. +Class `com.mysql.cj.jdbc.MysqlConnectionPoolDataSource` is present in the MySQL JDBC driver file, `mysql-connector-java-.jar`, which must be available on the server's classpath . + +If the class is instead present _within_ the web application, then the JNDI entry must be declared in a `WEB-INF/jetty-env.xml` file - see the link:#og-jndi[JNDI] feature section for more information and examples. -File `mysql-connector-java-.jar` must be either present in `WEB-INF/lib`, or in the Jetty server class-path. ==== // TODO: add a link to reference the section about how // to add a JDBC driver jar to the server classpath. diff --git a/jetty-documentation/src/main/asciidoc/operations-guide/jndi/chapter.adoc b/jetty-documentation/src/main/asciidoc/operations-guide/jndi/chapter.adoc index fc315f4cd6b..53935565647 100644 --- a/jetty-documentation/src/main/asciidoc/operations-guide/jndi/chapter.adoc +++ b/jetty-documentation/src/main/asciidoc/operations-guide/jndi/chapter.adoc @@ -329,14 +329,13 @@ Server XML file:: Naming resources defined in a server XML file are link:#og-jndi-scope[scoped] at either the JVM level or the `org.eclipse.jetty.server.Server` level. The classes for the resource _must_ be visible at the Jetty *container* level. If instead the classes for the resource only exist inside your webapp, you must declare it in a `WEB-INF/jetty-env.xml` file. +Context XML file:: +Entries in a context XML file should be link:#og-jndi-scope[scoped] at the level of the webapp to which they apply (although it is possible to use a less strict scoping level of Server or JVM, but not recommended). +As with resources declared in a server XML file, classes associated with the resource _must_ be visible on the *container's* classpath. WEB-INF/jetty-env.xml:: Naming resources in a `WEB-INF/jetty-env.xml` file are link:#og-jndi-scope[scoped] to the webapp in which the file resides. While you can enter JVM or Server scopes if you choose, we do not recommend doing so. The resources defined here may use classes from inside your webapp. -This is a Jetty-specific mechanism. -Context XML file:: -Entries in a context XML file should be link:#og-jndi-scope[scoped] at the level of the webapp to which they apply (although it is possible to use a less strict scoping level of Server or JVM, but not recommended). -As with resources declared in a server XML file, classes associated with the resource _must_ be visible on the *container's* classpath. [[og-jndi-scope]] ===== Resource scoping From 5e6083782201cde4eb9ff95e33bda3fcd0d17762 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Wed, 14 Oct 2020 23:31:24 +0200 Subject: [PATCH 3/3] Add more origin info to quickstart-web.xml elements (#5400) * Issue #5360 Add more origin info to quickstart-web.xml elements Signed-off-by: Jan Bartel --- .../DeclareRolesAnnotationHandler.java | 1 + .../annotations/WebFilterAnnotation.java | 4 +-- .../annotations/WebServletAnnotation.java | 4 ++- .../ExtraXmlDescriptorProcessor.java | 16 ++++++--- .../QuickStartGeneratorConfiguration.java | 10 +++--- .../webapp/StandardDescriptorProcessor.java | 36 +++++++++++-------- 6 files changed, 45 insertions(+), 26 deletions(-) diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/DeclareRolesAnnotationHandler.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/DeclareRolesAnnotationHandler.java index 72604d851ce..78197f9e9df 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/DeclareRolesAnnotationHandler.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/DeclareRolesAnnotationHandler.java @@ -63,6 +63,7 @@ public class DeclareRolesAnnotationHandler extends AbstractIntrospectableAnnotat for (String r : roles) { ((ConstraintSecurityHandler)_context.getSecurityHandler()).addRole(r); + _context.getMetaData().setOrigin("security-role." + r, declareRoles, clazz); } } } diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebFilterAnnotation.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebFilterAnnotation.java index bbad4bfadb7..1e445d64d25 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebFilterAnnotation.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebFilterAnnotation.java @@ -108,7 +108,7 @@ public class WebFilterAnnotation extends DiscoveredAnnotation FilterMapping mapping = new FilterMapping(); mapping.setFilterName(holder.getName()); - + metaData.setOrigin(name + ".filter.mapping." + Long.toHexString(mapping.hashCode()), filterAnnotation, clazz); if (urlPatterns.length > 0) { ArrayList paths = new ArrayList(); @@ -179,7 +179,7 @@ public class WebFilterAnnotation extends DiscoveredAnnotation { FilterMapping mapping = new FilterMapping(); mapping.setFilterName(holder.getName()); - + metaData.setOrigin(holder.getName() + ".filter.mapping." + Long.toHexString(mapping.hashCode()), filterAnnotation, clazz); if (urlPatterns.length > 0) { ArrayList paths = new ArrayList(); diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebServletAnnotation.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebServletAnnotation.java index 22d639a4645..547f68cd0cd 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebServletAnnotation.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebServletAnnotation.java @@ -154,6 +154,7 @@ public class WebServletAnnotation extends DiscoveredAnnotation mapping = new ServletMapping(source); mapping.setServletName(holder.getName()); mapping.setPathSpecs(LazyList.toStringArray(urlPatternList)); + _context.getMetaData().setOrigin(servletName + ".servlet.mapping." + Long.toHexString(mapping.hashCode()), annotation, clazz); } else { @@ -190,6 +191,7 @@ public class WebServletAnnotation extends DiscoveredAnnotation mapping = new ServletMapping(new Source(Source.Origin.ANNOTATION, clazz.getName())); mapping.setServletName(servletName); mapping.setPathSpecs(LazyList.toStringArray(urlPatternList)); + _context.getMetaData().setOrigin(servletName + ".servlet.mapping." + Long.toHexString(mapping.hashCode()), annotation, clazz); } } @@ -228,7 +230,7 @@ public class WebServletAnnotation extends DiscoveredAnnotation LOG.debug("Removed path {} from mapping {} from defaults descriptor ", p, existingMapping); } } - _context.getMetaData().setOrigin(servletName + ".servlet.mapping." + p, annotation, clazz); + _context.getMetaData().setOrigin(servletName + ".servlet.mapping.url" + p, annotation, clazz); } allMappings.add(mapping); _context.getServletHandler().setServletMappings(allMappings.toArray(new ServletMapping[allMappings.size()])); diff --git a/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/ExtraXmlDescriptorProcessor.java b/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/ExtraXmlDescriptorProcessor.java index 9e4f29d695f..6fae9d5442d 100644 --- a/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/ExtraXmlDescriptorProcessor.java +++ b/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/ExtraXmlDescriptorProcessor.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.quickstart; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.webapp.Descriptor; import org.eclipse.jetty.webapp.IterativeDescriptorProcessor; import org.eclipse.jetty.webapp.WebAppContext; @@ -36,12 +37,11 @@ public class ExtraXmlDescriptorProcessor extends IterativeDescriptorProcessor private static final Logger LOG = LoggerFactory.getLogger(ExtraXmlDescriptorProcessor.class); private final StringBuilder _buffer = new StringBuilder(); - private final boolean _showOrigin; + private String _originAttribute; private String _origin; public ExtraXmlDescriptorProcessor() { - _showOrigin = LOG.isDebugEnabled(); try { registerVisitor("env-entry", getClass().getMethod("saveSnippet", __signature)); @@ -60,7 +60,7 @@ public class ExtraXmlDescriptorProcessor extends IterativeDescriptorProcessor public void start(WebAppContext context, Descriptor descriptor) { LOG.debug("process {}", descriptor); - _origin = (" \n"); + _origin = (StringUtil.isBlank(_originAttribute) ? null : " \n"); } @Override @@ -68,11 +68,19 @@ public class ExtraXmlDescriptorProcessor extends IterativeDescriptorProcessor { } + public void setOriginAttribute(String name) + { + _originAttribute = name; + } + public void saveSnippet(WebAppContext context, Descriptor descriptor, XmlParser.Node node) throws Exception { + //Note: we have to output the origin as a comment field instead of + //as an attribute like the other other elements because + //we are copying these elements _verbatim_ from the descriptor LOG.debug("save {}", node.getTag()); - if (_showOrigin) + if (_origin != null) _buffer.append(_origin); _buffer.append(" ").append(node.toString()).append("\n"); } diff --git a/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartGeneratorConfiguration.java b/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartGeneratorConfiguration.java index b1530eaa1b1..1296e945c0d 100644 --- a/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartGeneratorConfiguration.java +++ b/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartGeneratorConfiguration.java @@ -241,7 +241,7 @@ public class QuickStartGeneratorConfiguration extends AbstractConfiguration if (f != null && f.getSource() == Source.EMBEDDED) continue; - out.openTag("filter-mapping"); + out.openTag("filter-mapping", origin(md, mapping.getFilterName() + ".filter.mapping." + Long.toHexString(mapping.hashCode()))); out.tag("filter-name", mapping.getFilterName()); if (mapping.getPathSpecs() != null) for (String s : mapping.getPathSpecs()) @@ -289,7 +289,7 @@ public class QuickStartGeneratorConfiguration extends AbstractConfiguration if (sh != null && sh.getSource() == Source.EMBEDDED) continue; - out.openTag("servlet-mapping", origin(md, mapping.getServletName() + ".servlet.mappings")); + out.openTag("servlet-mapping", origin(md, mapping.getServletName() + ".servlet.mapping." + Long.toHexString(mapping.hashCode()))); out.tag("servlet-name", mapping.getServletName()); if (mapping.getPathSpecs() != null) for (String s : mapping.getPathSpecs()) @@ -327,7 +327,7 @@ public class QuickStartGeneratorConfiguration extends AbstractConfiguration ConstraintAware ca = (ConstraintAware)security; for (String r : ca.getRoles()) { - out.openTag("security-role") + out.openTag("security-role", origin(md, "security-role." + r)) .tag("role-name", r) .closeTag(); } @@ -398,7 +398,7 @@ public class QuickStartGeneratorConfiguration extends AbstractConfiguration out.openTag("welcome-file-list"); for (String welcomeFile : context.getWelcomeFiles()) { - out.tag("welcome-file", welcomeFile); + out.tag("welcome-file", origin(md, "welcome-file." + welcomeFile), welcomeFile); } out.closeTag(); } @@ -429,6 +429,7 @@ public class QuickStartGeneratorConfiguration extends AbstractConfiguration if (cookieConfig != null) { out.openTag("cookie-config"); + if (cookieConfig.getName() != null) out.tag("name", origin(md, "cookie-config.name"), cookieConfig.getName()); @@ -789,6 +790,7 @@ public class QuickStartGeneratorConfiguration extends AbstractConfiguration public void preConfigure(WebAppContext context) throws Exception { ExtraXmlDescriptorProcessor extraXmlProcessor = new ExtraXmlDescriptorProcessor(); + extraXmlProcessor.setOriginAttribute(getOriginAttribute()); context.getMetaData().addDescriptorProcessor(extraXmlProcessor); context.setAttribute(ExtraXmlDescriptorProcessor.class.getName(), extraXmlProcessor); super.preConfigure(context); diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java index 09f8a865398..2e4a24d0c30 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java @@ -663,6 +663,7 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor if (TimeUnit.MINUTES.toSeconds(mins) > Integer.MAX_VALUE) throw new IllegalStateException("Max session-timeout in minutes is " + TimeUnit.SECONDS.toMinutes(Integer.MAX_VALUE)); context.getServletContext().setSessionTimeout((int)mins); + context.getMetaData().setOrigin("session.timeout", descriptor); } //Servlet Spec 3.0 @@ -672,7 +673,7 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor if (iter.hasNext()) { Set modes = null; - Origin o = context.getMetaData().getOrigin("session.tracking-mode"); + Origin o = context.getMetaData().getOrigin("session.tracking-modes"); switch (o) { case NotSet://not previously set, starting fresh @@ -680,7 +681,7 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor { modes = new HashSet(); - context.getMetaData().setOrigin("session.tracking-mode", descriptor); + context.getMetaData().setOrigin("session.tracking-modes", descriptor); break; } case WebXml: @@ -692,7 +693,7 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor modes = new HashSet(); else modes = new HashSet(context.getSessionHandler().getEffectiveSessionTrackingModes()); - context.getMetaData().setOrigin("session.tracking-mode", descriptor); + context.getMetaData().setOrigin("session.tracking-modes", descriptor); break; } default: @@ -703,7 +704,9 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor { XmlParser.Node mNode = (XmlParser.Node)iter.next(); String trackMode = mNode.toString(false, true); - modes.add(SessionTrackingMode.valueOf(trackMode)); + SessionTrackingMode mode = SessionTrackingMode.valueOf(trackMode); + modes.add(mode); + context.getMetaData().setOrigin("session.tracking-mode." + mode, descriptor); } context.getSessionHandler().setSessionTrackingModes(modes); } @@ -1035,13 +1038,13 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor case NotSet: { context.getMetaData().setOrigin("welcome-file-list", descriptor); - addWelcomeFiles(context, node); + addWelcomeFiles(context, node, descriptor); break; } case WebXml: { //web.xml set the welcome-file-list, all other descriptors then just merge in - addWelcomeFiles(context, node); + addWelcomeFiles(context, node, descriptor); break; } case WebDefaults: @@ -1052,19 +1055,19 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor { context.setWelcomeFiles(new String[0]); } - addWelcomeFiles(context, node); + addWelcomeFiles(context, node, descriptor); break; } case WebOverride: { //web-override set the list, all other descriptors just merge in - addWelcomeFiles(context, node); + addWelcomeFiles(context, node, descriptor); break; } case WebFragment: { //A web-fragment first set the welcome-file-list. Other descriptors just add. - addWelcomeFiles(context, node); + addWelcomeFiles(context, node, descriptor); break; } default: @@ -1182,14 +1185,14 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor } } - public void addWelcomeFiles(WebAppContext context, XmlParser.Node node) + public void addWelcomeFiles(WebAppContext context, XmlParser.Node node, Descriptor descriptor) { Iterator iter = node.iterator("welcome-file"); while (iter.hasNext()) { XmlParser.Node indexNode = (XmlParser.Node)iter.next(); String welcome = indexNode.toString(false, true); - + context.getMetaData().setOrigin("welcome-file." + welcome, descriptor); //Servlet Spec 3.0 p. 74 welcome files are additive if (welcome != null && welcome.trim().length() > 0) context.setWelcomeFiles((String[])ArrayUtil.addToArray(context.getWelcomeFiles(), welcome, String.class)); @@ -1201,7 +1204,8 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor ServletMapping mapping = new ServletMapping(new Source(Source.Origin.DESCRIPTOR, descriptor.getResource().toString())); mapping.setServletName(servletName); mapping.setFromDefaultDescriptor(descriptor instanceof DefaultsDescriptor); - + context.getMetaData().setOrigin(servletName + ".servlet.mapping." + Long.toHexString(mapping.hashCode()), descriptor); + List paths = new ArrayList(); Iterator iter = node.iterator("url-pattern"); while (iter.hasNext()) @@ -1255,7 +1259,7 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor } paths.add(p); - context.getMetaData().setOrigin(servletName + ".servlet.mapping." + p, descriptor); + context.getMetaData().setOrigin(servletName + ".servlet.mapping.url" + p, descriptor); } mapping.setPathSpecs((String[])paths.toArray(new String[paths.size()])); @@ -1269,7 +1273,7 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor { FilterMapping mapping = new FilterMapping(); mapping.setFilterName(filterName); - + context.getMetaData().setOrigin(filterName + ".filter.mapping." + Long.toHexString(mapping.hashCode()), descriptor); List paths = new ArrayList(); Iterator iter = node.iterator("url-pattern"); while (iter.hasNext()) @@ -1277,7 +1281,7 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor String p = iter.next().toString(false, true); p = ServletPathSpec.normalize(p); paths.add(p); - context.getMetaData().setOrigin(filterName + ".filter.mapping." + p, descriptor); + context.getMetaData().setOrigin(filterName + ".filter.mapping.url" + p, descriptor); } mapping.setPathSpecs((String[])paths.toArray(new String[paths.size()])); @@ -1286,6 +1290,7 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor while (iter.hasNext()) { String n = ((XmlParser.Node)iter.next()).toString(false, true); + context.getMetaData().setOrigin(filterName + ".filter.mapping.servlet" + n, descriptor); names.add(n); } mapping.setServletNames((String[])names.toArray(new String[names.size()])); @@ -1741,6 +1746,7 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor XmlParser.Node roleNode = node.get("role-name"); String role = roleNode.toString(false, true); ((ConstraintAware)context.getSecurityHandler()).addRole(role); + context.getMetaData().setOrigin("security-role." + role, descriptor); } public void visitFilter(WebAppContext context, Descriptor descriptor, XmlParser.Node node)