From 220896a0cf80ec349bda858fde8c1254a6af43e9 Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Sun, 15 Sep 2019 20:48:15 -0400 Subject: [PATCH 1/3] Improved logging for DefaultDispatcherErrorHandler and DefaultStaticContentLoader. Provide informational logging for the processing of the two dispatcher classes when an http sendError fails for the two known failure types (IOException, IllegalStateException). In both circumstances it is beneficial to have the developer aware of the failures via the logs. These are not events that should be happening on a regular basis, so there is little risk of a log flood. DefaultStaticContentLoader didn't catch either exception type previously, so changing it to make handling the same as DefaultDispatcherErrorHandler. For DefaultDispatcherErrorHandler the IOException was caught with no notice of failure previously. Now there will be an info level log output. Previously the IllegalStateException was not caught, which resulted in the unrecoverable exception being thrown up to the calling thread, usually resulting in an ugly stacktrace to stdout/stderr. Now there will be an info level log output instead. If devMode is true, the info log outputs will include the exception parameter to the log, so a stacktrace can be viewed for more details. If devMode is false (production mode), then only the exception's tostring() output will be produced. --- .../dispatcher/DefaultDispatcherErrorHandler.java | 8 ++++++++ .../struts2/dispatcher/DefaultStaticContentLoader.java | 10 +++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java index 07697714f..4e2af85da 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java @@ -97,6 +97,10 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler { response.sendError(code, e.getMessage()); } catch (IOException e1) { // we're already sending an error, not much else we can do if more stuff breaks + LOG.info("Unable to send error response, code: " + code + "! (IOException): " + e1); + } catch (IllegalStateException ise) { + // Log illegalstate instead of passing unrecoverable exception to calling thread + LOG.info("Unable to send error response, code: " + code + "! isCommited: " + response.isCommitted() + " (IllegalStateException): " + ise); } } @@ -122,6 +126,10 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler { response.sendError(code, "Unable to show problem report:\n" + exp + "\n\n" + LocationUtils.getLocation(exp)); } catch (IOException ex) { // we're already sending an error, not much else we can do if more stuff breaks + LOG.info("Unable to send error response, code: " + code + "! (IOException): ", ex); // Stacktrace with DevMode + } catch (IllegalStateException ise) { + // Log illegalstate instead of passing unrecoverable exception to calling thread + LOG.info("Unable to send error response, code: " + code + "! isCommited: " + response.isCommitted() + " (IllegalStateException): ", ise); // Stacktrace with DevMode } } } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java index eeedbf543..05a5f1b33 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java @@ -218,7 +218,15 @@ public class DefaultStaticContentLoader implements StaticContentLoader { } } - response.sendError(HttpServletResponse.SC_NOT_FOUND); + try { + response.sendError(HttpServletResponse.SC_NOT_FOUND); + } catch (IOException e1) { + // we're already sending an error, not much else we can do if more stuff breaks + LOG.info("Unable to send error response, code: " + HttpServletResponse.SC_NOT_FOUND + "! (IOException): " + e1); + } catch (IllegalStateException ise) { + // Log illegalstate instead of passing unrecoverable exception to calling thread + LOG.info("Unable to send error response, code: " + HttpServletResponse.SC_NOT_FOUND + "! isCommited: " + response.isCommitted() + " (IllegalStateException): " + ise); + } } protected void process(InputStream is, String path, HttpServletRequest request, HttpServletResponse response) throws IOException { From fcbd29b7f926e9c1c4775afec7ae1aded94ac8bb Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Tue, 17 Sep 2019 21:10:34 -0400 Subject: [PATCH 2/3] Changed the log outputs in this PR to utilize log4j2 {} notation (as per suggestion by A. Mashchenko and L. Lenart) Changed the log outputs in this PR from info level to warn level (as per suggestion by L. Lenart). Note: The info level was originally chosen to make it easier to filter out in the very unlikely case of log flood. --- .../struts2/dispatcher/DefaultDispatcherErrorHandler.java | 8 ++++---- .../struts2/dispatcher/DefaultStaticContentLoader.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java index 4e2af85da..5f1fd2272 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java @@ -97,10 +97,10 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler { response.sendError(code, e.getMessage()); } catch (IOException e1) { // we're already sending an error, not much else we can do if more stuff breaks - LOG.info("Unable to send error response, code: " + code + "! (IOException): " + e1); + LOG.warn("Unable to send error response, code: {}! (IOException): {}", code, e1.toString()); } catch (IllegalStateException ise) { // Log illegalstate instead of passing unrecoverable exception to calling thread - LOG.info("Unable to send error response, code: " + code + "! isCommited: " + response.isCommitted() + " (IllegalStateException): " + ise); + LOG.warn("Unable to send error response, code: {}! isCommited: {}. (IllegalStateException): {}", code, response.isCommitted(), ise.toString()); } } @@ -126,10 +126,10 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler { response.sendError(code, "Unable to show problem report:\n" + exp + "\n\n" + LocationUtils.getLocation(exp)); } catch (IOException ex) { // we're already sending an error, not much else we can do if more stuff breaks - LOG.info("Unable to send error response, code: " + code + "! (IOException): ", ex); // Stacktrace with DevMode + LOG.warn("Unable to send error response, code: {}! (IOException): {}", code, ex); // Stacktrace with DevMode } catch (IllegalStateException ise) { // Log illegalstate instead of passing unrecoverable exception to calling thread - LOG.info("Unable to send error response, code: " + code + "! isCommited: " + response.isCommitted() + " (IllegalStateException): ", ise); // Stacktrace with DevMode + LOG.warn("Unable to send error response, code: {}! isCommited: {}. (IllegalStateException): {}", code, response.isCommitted(), ise); // Stacktrace with DevMode } } } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java index 05a5f1b33..79c417918 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java @@ -222,10 +222,10 @@ public class DefaultStaticContentLoader implements StaticContentLoader { response.sendError(HttpServletResponse.SC_NOT_FOUND); } catch (IOException e1) { // we're already sending an error, not much else we can do if more stuff breaks - LOG.info("Unable to send error response, code: " + HttpServletResponse.SC_NOT_FOUND + "! (IOException): " + e1); + LOG.warn("Unable to send error response, code: {}! (IOException): {}", HttpServletResponse.SC_NOT_FOUND, e1.toString()); } catch (IllegalStateException ise) { // Log illegalstate instead of passing unrecoverable exception to calling thread - LOG.info("Unable to send error response, code: " + HttpServletResponse.SC_NOT_FOUND + "! isCommited: " + response.isCommitted() + " (IllegalStateException): " + ise); + LOG.warn("Unable to send error response, code: {}! isCommited: {}. (IllegalStateException): {}", HttpServletResponse.SC_NOT_FOUND, response.isCommitted(), ise.toString()); } } From 73809eae54e8a3fbcc6ee03d49379848cf7f99ed Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Sat, 21 Sep 2019 23:29:24 -0400 Subject: [PATCH 3/3] Changed the styling of log outputs (as suggested by A. Mashchenko). Changed the log outputs to output full stacktraces (as suggested by A. Mashchenko and L. Lenart). Updated existing DefaultStaticContentLoaderTest and created a new DefaultDispatcherErrorHandlerTest to supply tests that exercise the new code paths and demonstrate expected logging. --- .../DefaultDispatcherErrorHandler.java | 8 +- .../DefaultStaticContentLoader.java | 4 +- .../DefaultDispatcherErrorHandlerTest.java | 138 ++++++++++++++++++ .../DefaultStaticContentLoaderTest.java | 62 ++++++++ 4 files changed, 206 insertions(+), 6 deletions(-) create mode 100644 core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java index 5f1fd2272..551ff1e6a 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java @@ -97,10 +97,10 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler { response.sendError(code, e.getMessage()); } catch (IOException e1) { // we're already sending an error, not much else we can do if more stuff breaks - LOG.warn("Unable to send error response, code: {}! (IOException): {}", code, e1.toString()); + LOG.warn("Unable to send error response, code: {};", code, e1); } catch (IllegalStateException ise) { // Log illegalstate instead of passing unrecoverable exception to calling thread - LOG.warn("Unable to send error response, code: {}! isCommited: {}. (IllegalStateException): {}", code, response.isCommitted(), ise.toString()); + LOG.warn("Unable to send error response, code: {}; isCommited: {};", code, response.isCommitted(), ise); } } @@ -126,10 +126,10 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler { response.sendError(code, "Unable to show problem report:\n" + exp + "\n\n" + LocationUtils.getLocation(exp)); } catch (IOException ex) { // we're already sending an error, not much else we can do if more stuff breaks - LOG.warn("Unable to send error response, code: {}! (IOException): {}", code, ex); // Stacktrace with DevMode + LOG.warn("Unable to send error response, code: {};", code, ex); } catch (IllegalStateException ise) { // Log illegalstate instead of passing unrecoverable exception to calling thread - LOG.warn("Unable to send error response, code: {}! isCommited: {}. (IllegalStateException): {}", code, response.isCommitted(), ise); // Stacktrace with DevMode + LOG.warn("Unable to send error response, code: {}; isCommited: {};", code, response.isCommitted(), ise); } } } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java index 79c417918..42f17160e 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java @@ -222,10 +222,10 @@ public class DefaultStaticContentLoader implements StaticContentLoader { response.sendError(HttpServletResponse.SC_NOT_FOUND); } catch (IOException e1) { // we're already sending an error, not much else we can do if more stuff breaks - LOG.warn("Unable to send error response, code: {}! (IOException): {}", HttpServletResponse.SC_NOT_FOUND, e1.toString()); + LOG.warn("Unable to send error response, code: {};", HttpServletResponse.SC_NOT_FOUND, e1); } catch (IllegalStateException ise) { // Log illegalstate instead of passing unrecoverable exception to calling thread - LOG.warn("Unable to send error response, code: {}! isCommited: {}. (IllegalStateException): {}", HttpServletResponse.SC_NOT_FOUND, response.isCommitted(), ise.toString()); + LOG.warn("Unable to send error response, code: {}; isCommited: {};", HttpServletResponse.SC_NOT_FOUND, response.isCommitted(), ise); } } diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java new file mode 100644 index 000000000..e89dd3913 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java @@ -0,0 +1,138 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.dispatcher; + +import java.io.IOException; +import java.util.Collections; +import org.apache.struts2.StrutsInternalTestCase; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import org.apache.struts2.views.freemarker.FreemarkerManager; +import static org.easymock.EasyMock.anyInt; +import static org.easymock.EasyMock.anyString; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; +import static org.easymock.EasyMock.replay; + +public class DefaultDispatcherErrorHandlerTest extends StrutsInternalTestCase { + private HttpServletRequest requestMock; + private HttpServletResponse responseMock; + + /** + * Test to exercise the code path and prove handleError() will output + * the desired log warning when an IOException is thrown with devMode false. + */ + public void testHandleErrorIOException() { + DefaultDispatcherErrorHandler defaultDispatcherErrorHandler = new DefaultDispatcherErrorHandler(); + defaultDispatcherErrorHandler.setDevMode("false"); + defaultDispatcherErrorHandler.setFreemarkerManager(dispatcher.getContainer().getInstance(FreemarkerManager.class)); + defaultDispatcherErrorHandler.init(dispatcher.servletContext); + Exception fakeException = new Exception("Fake Exception, devMode false"); + try { + requestMock.setAttribute("javax.servlet.error.exception", fakeException); + expectLastCall(); + requestMock.setAttribute("javax.servlet.jsp.jspException", fakeException); + expectLastCall(); + responseMock.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, fakeException.getMessage()); + expectLastCall().andStubThrow(new IOException("Fake IO Exception (SC_INTERNAL_SERVER_ERROR, devMode false)")); + replay(responseMock); + } catch (IOException ioe) { + fail("Mock sendError call setup failed. Ex: " + ioe); + } + defaultDispatcherErrorHandler.handleError(requestMock, responseMock, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, fakeException); + } + + /** + * Test to exercise the code path and prove handleError() will output + * the desired log warning when an IOException is thrown with devMode true. + */ + public void testHandleErrorIOExceptionDevMode() { + DefaultDispatcherErrorHandler defaultDispatcherErrorHandler = new DefaultDispatcherErrorHandler(); + defaultDispatcherErrorHandler.setDevMode("true"); + defaultDispatcherErrorHandler.setFreemarkerManager(dispatcher.getContainer().getInstance(FreemarkerManager.class)); + defaultDispatcherErrorHandler.init(dispatcher.servletContext); + Exception fakeException = new Exception("Fake Exception, devMode true"); + try { + responseMock.setContentType("text/html"); + expectLastCall().andStubThrow(new IllegalStateException("Fake IllegalState Exception (report write)")); // Fake error during report write + responseMock.sendError(anyInt(), anyString()); + expectLastCall().andStubThrow(new IOException("Fake IO Exception (SC_INTERNAL_SERVER_ERROR, devMode true)")); + replay(responseMock); + } catch (IOException ioe) { + fail("Mock sendError call setup failed. Ex: " + ioe); + } + defaultDispatcherErrorHandler.handleError(requestMock, responseMock, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, fakeException); + } + + /** + * Test to exercise the code path and prove handleError() will output + * the desired log warning when an IllegalStateException is thrown with devMode false. + */ + public void testHandleErrorIllegalStateException() { + DefaultDispatcherErrorHandler defaultDispatcherErrorHandler = new DefaultDispatcherErrorHandler(); + defaultDispatcherErrorHandler.setDevMode("false"); + defaultDispatcherErrorHandler.setFreemarkerManager(dispatcher.getContainer().getInstance(FreemarkerManager.class)); + defaultDispatcherErrorHandler.init(dispatcher.servletContext); + Exception fakeException = new Exception("Fake Exception, devMode false"); + try { + requestMock.setAttribute("javax.servlet.error.exception", fakeException); + expectLastCall(); + requestMock.setAttribute("javax.servlet.jsp.jspException", fakeException); + expectLastCall(); + expect(responseMock.isCommitted()).andStubReturn(Boolean.TRUE); + responseMock.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, fakeException.getMessage()); + expectLastCall().andStubThrow(new IllegalStateException("Fake IllegalState Exception (SC_INTERNAL_SERVER_ERROR, devMode false)")); + replay(responseMock); + } catch (IOException ioe) { + fail("Mock sendError call setup failed. Ex: " + ioe); + } + defaultDispatcherErrorHandler.handleError(requestMock, responseMock, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, fakeException); + } + + /** + * Test to exercise the code path and prove handleError() will output + * the desired log warning when an IllegalStateException is thrown with devMode true. + */ + public void testHandleErrorIllegalStateExceptionDevMode() { + DefaultDispatcherErrorHandler defaultDispatcherErrorHandler = new DefaultDispatcherErrorHandler(); + defaultDispatcherErrorHandler.setDevMode("true"); + defaultDispatcherErrorHandler.setFreemarkerManager(dispatcher.getContainer().getInstance(FreemarkerManager.class)); + defaultDispatcherErrorHandler.init(dispatcher.servletContext); + Exception fakeException = new Exception("Fake Exception, devMode true"); + try { + expect(responseMock.isCommitted()).andStubReturn(Boolean.TRUE); + responseMock.setContentType("text/html"); + expectLastCall().andStubThrow(new IllegalStateException("Fake IllegalState Exception (report write)")); // Fake error during report write + responseMock.sendError(anyInt(), anyString()); + expectLastCall().andStubThrow(new IllegalStateException("Fake IllegalState Exception (SC_INTERNAL_SERVER_ERROR, devMode true)")); + replay(responseMock); + } catch (IOException ioe) { + fail("Mock sendError call setup failed. Ex: " + ioe); + } + defaultDispatcherErrorHandler.handleError(requestMock, responseMock, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, fakeException); + } + + protected void setUp() { + requestMock = (HttpServletRequest) createMock(HttpServletRequest.class); + responseMock = (HttpServletResponse) createMock(HttpServletResponse.class); + dispatcher = initDispatcher(Collections.emptyMap()); + } +} diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java index e8d077df0..8f8eda1eb 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java @@ -18,11 +18,22 @@ */ package org.apache.struts2.dispatcher; +import java.io.IOException; import org.apache.struts2.StrutsInternalTestCase; import java.util.List; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; +import static org.easymock.EasyMock.replay; public class DefaultStaticContentLoaderTest extends StrutsInternalTestCase { + private HttpServletRequest requestMock; + private HttpServletResponse responseMock; + private HostConfig hostConfigMock; + private DefaultStaticContentLoader defaultStaticContentLoader; public void testParsePackages() throws Exception { @@ -50,4 +61,55 @@ public class DefaultStaticContentLoaderTest extends StrutsInternalTestCase { assertEquals(result4.get(3), "foo/bar/package4/"); } + /** + * Test to exercise the code path and prove findStaticResource() will output + * the desired log warning when an IOException is thrown. + */ + public void testFindStaticResourceIOException() { + expect(requestMock.getDateHeader("If-Modified-Since")).andStubReturn(0L); + try { + responseMock.sendError(HttpServletResponse.SC_NOT_FOUND); + expectLastCall().andStubThrow(new IOException("Fake IO Exception (SC_NOT_FOUND)")); + replay(responseMock); + } catch (IOException ioe) { + fail("Mock sendError call setup failed. Ex: " + ioe); + } + try { + defaultStaticContentLoader.findStaticResource("/static/fake.html", requestMock, responseMock); + } catch (IOException ioe) { + fail("DefaultStaticContentLoader.findStaticResource() call failed. Ex: " + ioe); + } + } + + /** + * Test to exercise the code path and prove findStaticResource() will output + * the desired log warning when an IllegalStateException is thrown. + */ + public void testFindStaticResourceIllegalStateException() { + expect(requestMock.getDateHeader("If-Modified-Since")).andStubReturn(0L); + try { + expect(responseMock.isCommitted()).andStubReturn(Boolean.TRUE); + responseMock.sendError(HttpServletResponse.SC_NOT_FOUND); + expectLastCall().andStubThrow(new IllegalStateException("Fake IllegalState Exception (SC_NOT_FOUND)")); + replay(responseMock); + } catch (IOException ioe) { + fail("Mock sendError call setup failed. Ex: " + ioe); + } + try { + defaultStaticContentLoader.findStaticResource("/static/fake.html", requestMock, responseMock); + } catch (IOException ioe) { + fail("DefaultStaticContentLoader.findStaticResource() call failed. Ex: " + ioe); + } + } + + protected void setUp() { + requestMock = (HttpServletRequest) createMock(HttpServletRequest.class); + responseMock = (HttpServletResponse) createMock(HttpServletResponse.class); + hostConfigMock = (HostConfig) createMock(HostConfig.class); + expect(hostConfigMock.getInitParameter("packages")).andStubReturn(null); + expect(hostConfigMock.getInitParameter("loggerFactory")).andStubReturn(null); + defaultStaticContentLoader = new DefaultStaticContentLoader(); + defaultStaticContentLoader.setHostConfig(hostConfigMock); + defaultStaticContentLoader.setEncoding("UTF-8"); + } }