diff --git a/opendj-dsml-servlet/pom.xml b/opendj-dsml-servlet/pom.xml index c79cc0de25..520fcc69a6 100644 --- a/opendj-dsml-servlet/pom.xml +++ b/opendj-dsml-servlet/pom.xml @@ -108,6 +108,20 @@ ${project.groupId}.${project.artifactId} + + + ${basedir}/src/test/resources + + + + resources + ${basedir}/resources/schema + + DSMLv2.xsd + + + diff --git a/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java b/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java index 74ea5daed3..df2ba810a6 100644 --- a/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java +++ b/opendj-dsml-servlet/src/main/java/org/opends/dsml/protocol/DSMLServlet.java @@ -406,14 +406,17 @@ public void doPost(HttpServletRequest req, HttpServletResponse res) is.mark(65536); } - // Create response in the beginning as it might be used if the parsing - // fails. + // This prologue batchResponse answers everything detected before the + // SOAP body is walked (credentials errors, unparseable XML): those + // errors are built before any batchRequest is known. Each batchRequest + // of the body gets a batchResponse of its own, collected in responses + // below. ObjectFactory objFactory = new ObjectFactory(); - BatchResponse batchResponse = objFactory.createBatchResponse(); - List> batchResponses = batchResponse.getBatchResponses(); + BatchResponse prologueResponse = objFactory.createBatchResponse(); + List> prologueResponses = prologueResponse.getBatchResponses(); - // Thi sis only used for building the response - Document doc = createSafeDocument(); + // One batchResponse per batchRequest of the SOAP body, in request order. + List responses = new ArrayList<>(); MessageFactory messageFactory = null; String messageContentType = null; @@ -428,7 +431,7 @@ public void doPost(HttpServletRequest req, HttpServletResponse res) } catch(SSLConnectionException e) { - batchResponses.add( + prologueResponses.add( createErrorResponse(objFactory, new LDAPException(LDAPResultCode.CLIENT_SIDE_CONNECT_ERROR, LocalizableMessage.raw( @@ -493,7 +496,7 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) // user/DN:password parsing error. Keep reading the headers: the // Content-Type may still be ahead, and it decides which SOAP // version the error is reported with. - batchResponses.add( + prologueResponses.add( createErrorResponse(objFactory, new LDAPException(LDAPResultCode.INVALID_CREDENTIALS, LocalizableMessage.raw(ex.getMessage())))); @@ -517,7 +520,7 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) } else { - batchResponses.add( + prologueResponses.add( createErrorResponse(objFactory, new LDAPException(LDAPResultCode.INVALID_CREDENTIALS, LocalizableMessage.raw("Invalid configured credentials.")))); @@ -531,19 +534,19 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) } else { // otherwise if DN or password is null, send back an error if (((!authenticationIsID && bindDN == null) || bindPassword == null) - && batchResponses.isEmpty()) { - batchResponses.add( + && prologueResponses.isEmpty()) { + prologueResponses.add( createErrorResponse(objFactory, new LDAPException(LDAPResultCode.INVALID_CREDENTIALS, LocalizableMessage.raw("Unable to retrieve credentials.")))); } } - if ( batchResponses.isEmpty() && req.getContentLengthLong() > requestMaxSize ) { + if ( prologueResponses.isEmpty() && req.getContentLengthLong() > requestMaxSize ) { // The declared size already exceeds the cap: reject the request before // anything reads the stream — the malformed Content-Type fallback below // SAX-parses the whole body to recover the requestID. - batchResponses.add(createErrorResponse(objFactory, requestSizeExceeded())); + prologueResponses.add(createErrorResponse(objFactory, requestSizeExceeded())); } if ( messageFactory == null ) { @@ -560,36 +563,36 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) { throw new ServletException(e.getMessage()); } - if ( batchResponses.isEmpty() ) { + if ( prologueResponses.isEmpty() ) { // Nothing has been read from the stream yet, so the SAX pass can still // recover the requestID and let the client correlate the reply. - batchResponses.add( + prologueResponses.add( createXMLParsingErrorResponse(is, objFactory, - batchResponse, + prologueResponse, "Content-Type does not match SOAP 1.1 or SOAP 1.2")); } } // if an error already occurred, the list is not empty - if ( batchResponses.isEmpty() ) { + if ( prologueResponses.isEmpty() ) { try { SOAPMessage message = messageFactory.createMessage(mimeHeaders, is); soapBody = message.getSOAPBody(); } catch (SOAPException ex) { // SOAP was unable to parse XML successfully - batchResponses.add(cappedStream.isLimitExceeded() + prologueResponses.add(cappedStream.isLimitExceeded() ? createErrorResponse(objFactory, requestSizeExceeded()) : createXMLParsingErrorResponse(is, objFactory, - batchResponse, + prologueResponse, String.valueOf(ex.getCause()))); } catch (IOException ex) { if ( ! cappedStream.isLimitExceeded() ) { throw ex; } // The body streamed past the cap: chunked, or a lying Content-Length. - batchResponses.add(createErrorResponse(objFactory, requestSizeExceeded())); + prologueResponses.add(createErrorResponse(objFactory, requestSizeExceeded())); } } @@ -607,28 +610,45 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) // (MAX_BATCH_REQUESTS), before the element is even schema-validated. // The cap is counted over all the elements of the body, whatever // their type, and its configured value is not echoed to the - // unauthenticated client. - batchResponses.add(createErrorResponse(objFactory, - new LDAPException(LDAPResultCode.UNWILLING_TO_PERFORM, - LocalizableMessage.raw("The SOAP body holds more elements than the configured" - + " maximum: the remaining elements were not attempted.")))); + // unauthenticated client. The error joins the last answered + // batchResponse rather than forming a root of its own: DSMLv2 + // expects a single batchResponse per SOAP body, and under the + // default cap of one a separate root would give every default + // deployment a two-root reply. The cap is validated positive, so + // at least one element was answered before it could be exceeded. + responses.get(responses.size() - 1).getBatchResponses().add( + createErrorResponse(objFactory, + new LDAPException(LDAPResultCode.UNWILLING_TO_PERFORM, + LocalizableMessage.raw("The SOAP body holds more elements than the configured" + + " maximum: the remaining elements were not attempted.")))); break; } // Parse and unmarshall the SOAP object - the implementation prevents the use of a // DOCTYPE and xincludes, so should be safe. There is no way to configure a more // restrictive parser. SOAPElement se = (SOAPElement) obj; + + // Each batchRequest of the SOAP body is answered with its own + // batchResponse: a shared one would merge the elements of every + // batch request and keep only the last requestID. + BatchResponse elementResponse = objFactory.createBatchResponse(); + List> elementResponses = + elementResponse.getBatchResponses(); + responses.add(elementResponse); + JAXBElement batchRequestElement = null; try { Unmarshaller unmarshaller = jaxbContext.createUnmarshaller(); unmarshaller.setSchema(schema); batchRequestElement = unmarshaller.unmarshal(se, BatchRequest.class); } catch (JAXBException e) { - // schema validation failed - batchResponses.add(createXMLParsingErrorResponse(is, - objFactory, - batchResponse, - String.valueOf(e))); + // schema validation failed. The requestID is read from the element + // itself: the SAX pass of createXMLParsingErrorResponse() would + // recover the one of the first batchRequest of the body. + String requestID = se.getAttribute("requestID"); + elementResponse.setRequestID(requestID.isEmpty() ? null : requestID); + elementResponses.add( + createMalformedRequestError(objFactory, String.valueOf(e))); } if ( batchRequestElement != null ) { boolean authzInBind = false; @@ -658,7 +678,7 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) } } // set requestID in response - batchResponse.setRequestID(batchRequest.getRequestID()); + elementResponse.setRequestID(batchRequest.getRequestID()); org.opends.server.types.Control proxyAuthzControl = null; boolean connected = false; @@ -681,13 +701,13 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) ResultCode code = ResultCodeFactory.create(objFactory, LDAPResultCode.SUCCESS); authResponse.setResultCode(code); - batchResponses.add( + elementResponses.add( objFactory.createBatchResponseAuthResponse(authResponse)); } connected = true; } catch (LDAPConnectionException e) { // if connection failed, return appropriate error response - batchResponses.add(createErrorResponse(objFactory, e)); + elementResponses.add(createErrorResponse(objFactory, e)); } if ( connected ) { List list = batchRequest.getBatchRequests(); @@ -698,7 +718,7 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) // an abandon request does not produce any response element continue; } - batchResponses.add(result); + elementResponses.add(result); // evaluate response to check if an error occurred Object o = result.getValue(); if ( o instanceof ErrorResponse ) { @@ -726,9 +746,24 @@ else if (headerVal.startsWith(SOAPConstants.SOAP_1_2_CONTENT_TYPE)) } } try { + if ( !prologueResponses.isEmpty() || responses.isEmpty() ) { + // An error was detected before the SOAP body was walked, or the body + // holds no batchRequest at all: reply with the prologue batchResponse. + // The guards above keep both lists from being populated at once + // today; prepending keeps the prologue errors visible even if a + // future edit changes that. + responses.add(0, prologueResponse); + } Marshaller marshaller = jaxbContext.createMarshaller(); - marshaller.marshal(objFactory.createBatchResponse(batchResponse), doc); - sendResponse(doc, messageFactory, messageContentType, res); + List docs = new ArrayList<>(responses.size()); + for (BatchResponse response : responses) { + // A DOM document has a single root element, so each batchResponse of + // the reply is marshalled into a document of its own. + Document doc = createSafeDocument(); + marshaller.marshal(objFactory.createBatchResponse(response), doc); + docs.add(doc); + } + sendResponse(docs, messageFactory, messageContentType, res); } catch (Exception e) { // The client gets an empty response: at least make the cause visible. getServletContext().log("Unable to send the DSML response", e); @@ -787,7 +822,6 @@ private JAXBElement createXMLParsingErrorResponse( ObjectFactory objFactory, BatchResponse batchResponse, String parserErrorMessage) { - ErrorResponse errorResponse = objFactory.createErrorResponse(); DSMLContentHandler contentHandler = new DSMLContentHandler(); try @@ -803,13 +837,27 @@ private JAXBElement createXMLParsingErrorResponse( { // ignore } - if ( parserErrorMessage!= null ) { - errorResponse.setMessage(parserErrorMessage); - } batchResponse.setRequestID(contentHandler.requestID); - errorResponse.setType(MALFORMED_REQUEST); + return createMalformedRequestError(objFactory, parserErrorMessage); + } + /** + * Returns an error response of type 'malformed request' carrying the given + * message, or none if it is {@code null}. + * + * @param objFactory the object factory + * @param message the error message, may be {@code null} + * + * @return a JAXBElement that contains an ErrorResponse + */ + private JAXBElement createMalformedRequestError( + ObjectFactory objFactory, String message) { + ErrorResponse errorResponse = objFactory.createErrorResponse(); + if ( message != null ) { + errorResponse.setMessage(message); + } + errorResponse.setType(MALFORMED_REQUEST); return objFactory.createBatchResponseErrorResponse(errorResponse); } @@ -974,7 +1022,8 @@ private JAXBElement performLDAPRequest(LDAPConnection connection, * Send a response back to the client. This could be either a SOAP fault * or a correct DSML response. * - * @param doc The document to include in the response. + * @param docs The documents to include in the response, one per + * batchResponse element of the reply. * @param messageFactory The SOAP message factory. * @param contentType The MIME content type to send appropriate for the MessageFactory * @param res Information about the HTTP response to the client. @@ -982,7 +1031,8 @@ private JAXBElement performLDAPRequest(LDAPConnection connection, * @throws IOException If an error occurs while interacting with the client. * @throws SOAPException If an encoding or decoding error occurs. */ - private void sendResponse(Document doc, MessageFactory messageFactory, String contentType, HttpServletResponse res) + private void sendResponse(List docs, MessageFactory messageFactory, String contentType, + HttpServletResponse res) throws IOException, SOAPException { SOAPMessage reply = messageFactory.createMessage(); @@ -992,7 +1042,9 @@ private void sendResponse(Document doc, MessageFactory messageFactory, String co res.setHeader("Content-Type", contentType); - replyBody.addDocument(doc); + for (Document doc : docs) { + replyBody.addDocument(doc); + } reply.saveChanges(); diff --git a/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java b/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java index d68b5f05be..98156609b3 100644 --- a/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java +++ b/opendj-dsml-servlet/src/test/java/org/opends/dsml/protocol/DSMLServletTestCase.java @@ -55,6 +55,7 @@ import jakarta.servlet.WriteListener; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import javax.xml.parsers.DocumentBuilderFactory; import org.forgerock.opendj.ldap.ByteString; import org.forgerock.testng.ForgeRockTestCase; @@ -66,15 +67,19 @@ import org.opends.server.tools.LDAPReader; import org.opends.server.tools.LDAPWriter; import org.testng.annotations.Test; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NodeList; /** * Tests the error handling of {@link DSMLServlet#doPost}: an abandon request * used to trigger a {@code NullPointerException} which leaked the LDAP * connection, a request without a usable Content-Type header used to trigger a - * {@code NullPointerException} as well, and the second batch request of a SOAP - * body used to be silently skipped. Also covers the caps on the number of - * batchRequest elements per SOAP body (each element costs a bind) and on the - * size of the request body. + * {@code NullPointerException} as well, the second batch request of a SOAP + * body used to be silently skipped, and every batch request of a SOAP body + * used to be answered inside a single shared batchResponse. Also covers the + * caps on the number of batchRequest elements per SOAP body (each element + * costs a bind) and on the size of the request body. */ @SuppressWarnings("javadoc") @Test(groups = { "precommit", "dsml" }) @@ -86,6 +91,8 @@ public class DSMLServletTestCase extends ForgeRockTestCase private static final String SOAP_1_2_CONTENT_TYPE = "application/soap+xml"; /** SOAP 1.2 envelope namespace, as it appears in the reply. */ private static final String SOAP_1_2_NAMESPACE = "http://www.w3.org/2003/05/soap-envelope"; + /** DSMLv2 core namespace. */ + private static final String DSML_NAMESPACE = "urn:oasis:names:tc:DSML:2:0:core"; private static final String ABANDON_BATCH = soap11(abandonBatch("1", null)); @@ -112,7 +119,7 @@ public class DSMLServletTestCase extends ForgeRockTestCase private static String abandonBatch(String requestID, String authzPrincipal) { - return "" + return "" + (authzPrincipal != null ? "" : "") + "" + ""; @@ -303,6 +310,9 @@ public void testEachBatchRequestGetsItsOwnConnection() throws Exception * bind, so by default a single POST may only hold one: the excess must be * rejected without being executed, not silently skipped, and the results of * the elements under the cap must still reach the client next to the error. + * The error joins the answered batchResponse instead of forming a root of + * its own: DSMLv2 expects a single batchResponse per SOAP body, and the + * default configuration must not produce a two-root reply. */ @Test public void testExcessBatchRequestsAreRejectedByDefault() throws Exception @@ -314,8 +324,15 @@ public void testExcessBatchRequestsAreRejectedByDefault() throws Exception String response = doPost(server.getPort(), headers, SEARCH_AND_ABANDON_BATCHES); - assertTrue(response.contains("searchResponse"), response); - assertTrue(response.contains("notAttempted"), response); + List replies = batchResponsesOf(response); + assertEquals(replies.size(), 1, + "the default configuration must answer with a single batchResponse root: " + response); + assertEquals(replies.get(0).getAttribute("requestID"), "1", response); + List elements = childElements(replies.get(0)); + assertEquals(elements.size(), 2, response); + assertEquals(elements.get(0).getLocalName(), "searchResponse", response); + assertEquals(elements.get(1).getLocalName(), "errorResponse", response); + assertEquals(elements.get(1).getAttribute("type"), "notAttempted", response); server.awaitDisconnect(); assertEquals(server.getReceivedOpTypes(), @@ -510,6 +527,185 @@ public void testAuthzIdDoesNotSurviveIntoBatchRequestWithoutAuthRequest() throws } } + /** + * Every batchRequest of a SOAP body is answered with a batchResponse of its + * own: a single shared one used to merge the elements of every batch + * request and to keep only the requestID of the last one. + */ + @Test + public void testEachBatchRequestGetsItsOwnBatchResponse() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + String response = doAuthzPost(server, TWO_AUTHZ_BATCHES); + + server.awaitDisconnect(2); + List replies = batchResponsesOf(response); + assertEquals(replies.size(), 2, response); + assertEquals(replies.get(0).getAttribute("requestID"), "1", response); + assertEquals(replies.get(1).getAttribute("requestID"), "2", response); + for (Element reply : replies) + { + List elements = childElements(reply); + assertEquals(elements.size(), 1, + "each batch request must keep its response elements to itself: " + response); + assertEquals(elements.get(0).getLocalName(), "authResponse", response); + } + } + } + + /** + * A batchRequest which fails schema validation is answered inside its own + * batchResponse, under its own requestID: the SAX fallback used to recover + * the requestID of the first batchRequest of the body instead. The cap on + * batchRequest elements has to be raised to let two of them in. + */ + @Test + public void testMalformedBatchRequestIsAnsweredUnderItsOwnRequestID() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + Map params = new LinkedHashMap<>(); + params.put("ldap.dsml.batchrequests.max", "2"); + + Map headers = new LinkedHashMap<>(); + headers.put("Content-Type", SOAP_1_1_CONTENT_TYPE); + + String malformed = "" + + ""; + String response = + doPost(server.getPort(), params, headers, soap11(abandonBatch("1", null) + malformed)); + + server.awaitDisconnect(); + List replies = batchResponsesOf(response); + assertEquals(replies.size(), 2, response); + assertEquals(replies.get(0).getAttribute("requestID"), "1", response); + assertTrue(childElements(replies.get(0)).isEmpty(), + "an abandon request produces no response element: " + response); + assertEquals(replies.get(1).getAttribute("requestID"), "2", response); + List errors = childElements(replies.get(1)); + assertEquals(errors.size(), 1, response); + assertEquals(errors.get(0).getLocalName(), "errorResponse", response); + assertEquals(errors.get(0).getAttribute("type"), "malformedRequest", response); + assertEquals(server.getReceivedOpTypes(), + list(OP_TYPE_BIND_REQUEST, OP_TYPE_ABANDON_REQUEST, OP_TYPE_UNBIND_REQUEST), + "only the valid batch request should have reached the directory server"); + } + } + + /** + * A malformed batchRequest without a requestID is answered inside a + * batchResponse which carries no requestID at all: an empty attribute would + * read as a requestID of "". + */ + @Test + public void testMalformedBatchRequestWithoutRequestIDIsAnsweredWithoutOne() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + Map headers = new LinkedHashMap<>(); + headers.put("Content-Type", SOAP_1_1_CONTENT_TYPE); + + String malformed = "" + + ""; + String response = doPost(server.getPort(), headers, soap11(malformed)); + + List replies = batchResponsesOf(response); + assertEquals(replies.size(), 1, response); + assertFalse(replies.get(0).hasAttribute("requestID"), + "a batch request without a requestID must be answered without one: " + response); + List errors = childElements(replies.get(0)); + assertEquals(errors.size(), 1, response); + assertEquals(errors.get(0).getLocalName(), "errorResponse", response); + assertEquals(errors.get(0).getAttribute("type"), "malformedRequest", response); + assertTrue(server.getReceivedOpTypes().isEmpty(), + "no connection to the directory server should have been opened"); + } + } + + /** + * A SOAP body element which is no batchRequest at all is answered in place, + * as a malformed request, under the requestID read from the element itself. + */ + @Test + public void testNonBatchRequestElementIsAnsweredAsMalformed() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + Map params = new LinkedHashMap<>(); + params.put("ldap.dsml.batchrequests.max", "2"); + + Map headers = new LinkedHashMap<>(); + headers.put("Content-Type", SOAP_1_1_CONTENT_TYPE); + + String alien = ""; + String response = + doPost(server.getPort(), params, headers, soap11(abandonBatch("1", null) + alien)); + + server.awaitDisconnect(); + List replies = batchResponsesOf(response); + assertEquals(replies.size(), 2, response); + assertEquals(replies.get(0).getAttribute("requestID"), "1", response); + assertEquals(replies.get(1).getAttribute("requestID"), "7", response); + List errors = childElements(replies.get(1)); + assertEquals(errors.size(), 1, response); + assertEquals(errors.get(0).getLocalName(), "errorResponse", response); + assertEquals(errors.get(0).getAttribute("type"), "malformedRequest", response); + assertEquals(server.getReceivedOpTypes(), + list(OP_TYPE_BIND_REQUEST, OP_TYPE_ABANDON_REQUEST, OP_TYPE_UNBIND_REQUEST), + "only the batchRequest should have reached the directory server"); + } + } + + /** A SOAP body without any batchRequest is answered with a single, empty batchResponse. */ + @Test + public void testEmptySoapBodyIsAnsweredWithEmptyBatchResponse() throws Exception + { + try (FakeLdapServer server = new FakeLdapServer()) + { + Map headers = new LinkedHashMap<>(); + headers.put("Content-Type", SOAP_1_1_CONTENT_TYPE); + + String response = doPost(server.getPort(), headers, soap11("")); + + List replies = batchResponsesOf(response); + assertEquals(replies.size(), 1, response); + assertTrue(childElements(replies.get(0)).isEmpty(), response); + assertTrue(server.getReceivedOpTypes().isEmpty(), + "no connection to the directory server should have been opened"); + } + } + + /** The batchResponse elements of the reply, in document order. */ + private static List batchResponsesOf(String response) throws Exception + { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setNamespaceAware(true); + Document doc = factory.newDocumentBuilder() + .parse(new ByteArrayInputStream(response.getBytes(StandardCharsets.UTF_8))); + NodeList nodes = doc.getElementsByTagNameNS(DSML_NAMESPACE, "batchResponse"); + List result = new ArrayList<>(); + for (int i = 0; i < nodes.getLength(); i++) + { + result.add((Element) nodes.item(i)); + } + return result; + } + + private static List childElements(Element parent) + { + List result = new ArrayList<>(); + NodeList children = parent.getChildNodes(); + for (int i = 0; i < children.getLength(); i++) + { + if (children.item(i) instanceof Element) + { + result.add((Element) children.item(i)); + } + } + return result; + } + /** * Posts the given SOAP body with HTTP credentials turned into a SASL PLAIN * authid, so that the authRequest of a batch request becomes an authzid.