[#824] Answer each batchRequest of a SOAP body with its own batchResponse - #836
Open
vharseko wants to merge 5 commits into
Open
Conversation
createSocket() has been binding the new client socket to the target server address instead of connecting to it since OpenIdentityPlatform#279, so every plain or StartTLS connection made through org.opends.server.tools.LDAPConnection fails with "Address already in use" (server on the same host) or "Cannot assign requested address" (remote server). Affects the DSML gateway, stop-ds, manage-account and the other tools built on LDAPConnectionArgumentParser.
… on missing Content-Type performLDAPRequest() returns null for an abandon request, but doPost() dereferenced the result unconditionally, so a batch containing <abandonRequest/> ended in a NullPointerException; as the connection was closed after the loop instead of in a finally, one LDAP connection was leaked per request. messageFactory was only assigned when a SOAP 1.1 or SOAP 1.2 Content-Type header was present, and was then dereferenced both when parsing the request and when sending the response: a POST without Content-Type ended in a NullPointerException, and, when an error response had already been queued, in an empty HTTP 200 instead of that error. A missing or unsupported Content-Type is now answered with a malformedRequest batch response. Also log the failure instead of printing the stack trace when the response cannot be sent, and add regression tests for both defects.
Report the failure to send the response to the container log: the java.util.logging record was dropped, as connectToHost() resets the LogManager and turns the root logger off on every non-verbose connection. This needs super.init(config), without which getServletContext() throws. Drop the authzid of the previous batch request before setting the new one: the connection options are shared by the whole SOAP body and addSASLProperty() appends to the values of a key, so a second authRequest made SASL PLAIN reject a multi-valued authzid. Now that the connection is never reused, make it a loop local and remove the dead null check that guarded the reuse. Build the malformed Content-Type response with createXMLParsingErrorResponse(), like the other two malformed paths, so that the requestID is recovered; and keep reading the headers after a malformed Authorization one, so that the reply keeps the SOAP version of the request. Cover the SOAP 1.2 path, the per-batch-request connection and the authzid, let the fake LDAP endpoint serve several connections and fail the test on a server-side error, and pin the createSocket() regression of OpenIdentityPlatform#279 with a test in the module that owns it.
…uest of a SOAP body The connection options are built once per doPost() and shared by all the batch requests of the SOAP body, but the authzid was dropped only when the next batch request carried an authRequest of its own. A body whose first batch request asks for an authorization identity and whose second does not left the first authzid in the options, so the operations of the second one ran under an identity the request never asked for. It is gated on ldap.authzidtypeisid=true, which the shipped web.xml leaves at false, and still subject to the proxied-auth privileges of the server. The clearing now happens at the top of every iteration, before the authRequest is looked at. DSMLServletTestCase records the authorization identity of every SASL bind at the fake endpoint: the existing test now asserts the identities themselves instead of the mere absence of an error, and a new one pins the mixed body, where the second batch request must bind with no authzid at all. The four remaining Logger.getLogger(PKG_NAME) calls are replaced by getServletContext().log(), so the class has a single logging sink: they were dead for the reason already documented for the response path, which moves to the class javadoc.
…th its own batchResponse A single BatchResponse was shared by every batchRequest of the SOAP body: the elements of all batch requests were merged into one reply and the requestID was overwritten on each iteration. Marshal one batchResponse per batchRequest into the SOAP body, keep the upfront instance for the errors detected before the body is walked, and answer a batchRequest which fails schema validation under its own requestID. The DSMLv2 schema is now on the test classpath: without it the unmarshaller silently skips validation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #824.
DSMLServlet.doPost()built a singleBatchResponsebefore walking the SOAP body: the response elements of everybatchRequestwere merged into one reply and therequestIDwas overwritten on each iteration, so a body holding two batch requests was answered with onebatchResponsecarrying the requestID of the last one and the elements of both.batchRequestof the SOAP body is now answered with abatchResponseof its own, marshalled into a document of its own (a DOM document has a single root element) and added to the same SOAP reply body.BatchResponseremains for the errors detected before the body is walked (credentials, unparseable XML, unusable Content-Type) and for a body without anybatchRequest: those replies are unchanged.batchRequestwhich fails schema validation is answered inside its ownbatchResponseunder its ownrequestID, read from the element itself: the SAX fallback would recover the requestID of the firstbatchRequestof the body instead.WEB-INF/classes/resources, so in unit testsschemawas silentlynulland validation never ran.DSMLServletTestCasegains three cases: each batch request keeps its response elements and requestID to itself, a malformed batch request is answered under its own requestID while the valid one still runs, and an empty SOAP body still gets a single emptybatchResponse. All 12 tests of the class pass, and the war still carries the schema.Stacked on #811: only the last commit belongs to this PR, the commits before it are that fix. I will rebase once #811 lands.