[maximthomas]

Summary

This is slice S4 of the sequential Restlet to CHF migration tracked in #1075. The /xacml policy import/export endpoint moves from the ForgeRockRest servlet (Restlet) to the OpenAM servlet (CHF), next to /json, and its Restlet wiring is deleted. The wire format is kept except for the divergences listed below.

Changes

• openam-http: Endpoints serves a HEAD request with the GET method and puts an Allow header on every 405 response (AnnotatedMethod.isSupported() added for this).
• openam-entitlements: XacmlService is a plain annotated CHF handler (@Post, @Get) instead of a Restlet resource.
• openam-entitlements: new XacmlHttpRouteProvider registers /xacml with CAF authentication, HttpContextFilter, the resource API version filter and a protocol 1.0 enforcement filter, and routes realms the way /json does (/xacml/policies, /xacml/<realm>/policies, /xacml/realms/root[/realms/<sub>]/policies).
• web.xml maps /xacml/* to the OpenAM servlet; RestEndpointServlet keeps only /oauth2 and /uma (its RestletAuthnHttpApplication, RestletHandler and HttpServletWrapper inner classes go) and answers 404 to any other servlet path, so a customized web.xml that still maps /xacml/* to ForgeRockRest gets 404, not an empty 200; six classes are deleted: XacmlRouterProvider, XACMLServiceEndpointApplication, XMLRestStatusService, ResourceApiVersionRestlet, ResourceApiVersionRoutingFilter, RestletRouter (plus XMLRestStatusServiceTest).
• e2e: e2e/xacml/xacml-test.spec.mjs moves to CHF's answers and gains three tests; the upgrade guide gets an [IMPORTANT] note.

Divergences produced by the CHF code

Request	master	branch
GET /xacml/realms/root/policies	404	200
GET /xacml/realms/root/realms/<sub>/policies	404	200
Content-API-Version value	[Content-API-Version: protocol=1.0,resource=1.0]	protocol=1.0,resource=1.0
import with an empty document	500	400
HEAD /xacml/policies	500	200, no body

HEAD cause on master (verified on a running master container): Restlet answers HEAD with 200 and its headers but no body, so the servlet response stays uncommitted. RestletHandler then returns null, CAF AuthenticationFramework.grantAccess throws an NPE, and HttpFrameworkServlet logs "RuntimeException caught" and writes Responses.newInternalServerError() (setStatus(500)) over the 200. For a GET the body has already committed the response, so the late 500 is ignored.

Unpinned changes

Measured on a master and a branch container with an admin token.

• Unknown ?realm=: 404 application/xml error -> 400 application/json (Invalid realm, /nosuchrealm).
• Unknown realm in the path (/xacml/nosuchrealm/policies): 404 XML -> 404 with an empty body.
• Unknown realm in the nested spelling (/xacml/realms/root/realms/nosuchrealm/policies): 404 XML (naming the segment /realms) -> 404 JSON (Realm "/nosuchrealm" not found).
• Invalid Accept-API-Version: resource=x: 500 XML -> 400 plain text (Version string is in an invalid format: resource=x).
• Routing-level error bodies are no longer the XML error document.
• PUT /xacml/policies: 405 with Allow: GET, POST on both; the body changes from XML to JSON and Content-API-Version loses the brackets.
• Other annotated endpoints that share Endpoints: HEAD /json/api was 405 and is now answered like GET; PUT /json/api 405 now carries Allow: GET; GET and PUT /json/authenticate 405 now carry Allow: POST. In the test stand GET /json/api itself answers 500 with an empty body on both builds, so HEAD shows 500 on the branch; that is existing ApiService behaviour, not part of this PR.
• No Server: Restlet-Framework/2.4.4 header on /xacml responses.
• <message> of the XML error documents that XacmlService writes: master's Restlet ResourceException carried Status.toString(), so an import without policies answered No policies found in XACML document (400); the branch writes No policies found in XACML document. The text of a failed import follows the request's Accept-Language instead of the server locale. Traced through the Restlet 2.4.4 bytecode (StatusService.getStatus, ResourceException(Status), RestStatusService), not measured; listed in the upgrade note.
• Content-Encoding of an import body: gzip is decoded on both (e2e test below). Restlet's DecoderService also accepted deflate in the zlib format, deflate-no-wrap, zip and identity (passed through); CHF 3.2.2 has decoders for gzip and deflate only, and its DeflateDecoder reads raw DEFLATE (new Inflater(true)). So a zlib deflate body answers 400 XML cannot be parsed. instead of being imported, a raw DEFLATE body sent as deflate (400 on master) is imported, and deflate-no-wrap, zip and identity answer 400 Unsupported Content-Encoding: <coding>. A coding neither side knows, such as br, answers 400 on both, XML cannot be parsed. on master and Unsupported Content-Encoding: br on the branch. Traced through the Restlet 2.4.4 and CHF 3.2.2 bytecode, with the zlib case reproduced in jshell (ZipException: invalid stored block lengths); not measured on a container. Listed in the upgrade note.

Known edges, not fixed here

CHF's global ApiDescriptorFilter calls request.getForm() on every request. An import posted with a bare Content-Type: application/x-www-form-urlencoded (for example curl --data @file.xml without -H) whose XML contains a % not followed by two hex digits answers 500 without a body. The console sends ; charset=UTF-8, which CHF does not parse as a form, so it is not affected (pinned by an e2e test whose document holds such a %; on the branch container the same body answers 500 with the bare content type and 200 with ; charset=UTF-8). Proposed follow-up: read _api with new Form().fromRequestQuery(request).

CHF's DeflateDecoder reads Content-Encoding: deflate as raw DEFLATE, where RFC 9110 section 8.4.1.2 defines the zlib format (see the Content-Encoding bullet above). Under the #1075 rule that CHF defects are fixed in commons, that fix is tracked in OpenIdentityPlatform/commons#385; XacmlService already decodes through CHF and picks it up.

Not changed (decision)

A request without a session still answers 500 XML. The 401 target is tracked separately.

Tests

Unit tests, all with 0 failures:

• openam-http: EndpointsTest 8 (module total 17)
• openam-restlet: module total 11
• openam-rest: 283 + 8
• openam-entitlements: XacmlServiceTest 25, XacmlHttpRouteProviderTest 8 (module total 616)

Four new e2e tests (23 in the spec):

• answers HEAD like GET without a body (a divergence: master 500)
• reads a document posted with the console's form content type (parity: passes on master; its document holds a % that two hex digits do not follow, so it fails if CHF ever parses the body as a form)
• resolves a sub realm under the realms/root path spelling (nested realms)
• reads a gzip-compressed document (parity: passes on master, where Restlet's DecoderService decoded the body)

Runtime evidence

Docker stand: openidentityplatform/openam image with the WAR replaced, embedded config store, xacml/xacml-test run twice on one container.

• master WAR: 16 passed / 6 failed in both runs (the five moved targets plus HEAD)
• branch WAR: 22 passed / 0 failed in both runs
• RuntimeException caught in debug/org.forgerock.http.servlet.HttpFrameworkServlet: master 68, branch 0 (the file is absent)
• production mutant (branch WAR with EnforcedProtocolVersionFilter removed from the chain): 20 passed / 2 failed in both runs; the two Content-API-Version tests fail with Received "resource=1.0"
• after the review fixes (branch WAR with the new XacmlService class patched in, master WAR, same spec): branch 22 passed / 0 failed and master 16 passed / 6 failed in both runs, the same six tests as above. On the branch container a realm named xq[1] exports with Content-Disposition: attachment; filename="xq[1]-realm-policies.xml", quoted as master's Restlet Disposition did; the root realm keeps attachment; filename=realm-policies.xml.
• after the review round 2 fix (gzip import), same spec of 23 tests: branch WAR with the new XacmlService class patched in 23 passed / 0 failed in both runs; the branch WAR before the fix 22 passed / 1 failed in both runs, only reads a gzip-compressed document (Expected 200, Received 400); master WAR 17 passed / 6 failed in both runs, the same six tests as above, so the gzip test passes on master.
• startup check on the branch container: no Guice or injection errors, no mention of HttpRouterProvider or XacmlHttpRouteProvider in catalina.out; /json/authenticate issued the admin token. The Entitlement debug log holds only policy-evaluation errors (a combining algorithm URN looked up as a class name, ClassNotFoundException) and dropped PolicySet notifications, as on master; none come from routing.

Upgrade note

New [IMPORTANT] block in openam-documentation/openam-doc-source/src/main/asciidoc/upgrade-guide/chap-upgrade-servers.adoc: a customized web.xml must move the /xacml/* mapping to the OpenAM servlet, the behaviour changes above, the removed classes and the removed @Named("XacmlRouter") Guice binding of the Restlet Router.

[vharseko]

Review of PR #1214 at head a639fdb.

[P2] Decode Content-Encoding before passing the import body to the XML parser

Location: openam-entitlements/src/main/java/org/forgerock/openam/xacml/v3/rest/XacmlService.java:153–154.

A POST to /xacml/policies with Content-Type: application/xml and Content-Encoding: gzip now passes compressed bytes directly to the XACML parser. getRawContentInputStream() deliberately performs no content decoding. The previous Restlet Application enabled DecoderService by default, so the Representation supplied to importXACML had already been decompressed. Clients that compress policy imports therefore lose working functionality after this migration: the new handler returns 400 with “XML cannot be parsed.” for otherwise valid XML.

I reproduced the transport/parser difference in an isolated Java harness using Restlet 2.4.4, this PR’s XacmlService, and the real XACMLReaderWriter. Restlet’s default application chain decoded gzip back to the original XML and the parser accepted it. With authorization and policy persistence stubbed, the new handler accepted the plain XML (200) but returned 400 for its gzip-compressed equivalent. This was not a full-server HTTP test.

Please read the body through Entity.newDecodedContentInputStream(), close that stream appropriately, and handle decoding/I/O failures as client errors. Add a regression case that posts a gzip-compressed valid policy document and checks the same import result as the uncompressed document. This regression is separate from the bare form-content-type issue already documented in the PR.

Validation:

• Read the PR description and complete discussion, plus #1075 and the related migration/characterization PRs #1201, #1202 and #1211. No published discussion on #1214 addresses compressed imports.
• 39 selected unit tests passed: EndpointsTest (8), RestEndpointServletTest (2 enabled tests), XacmlServiceTest (21), XacmlHttpRouteProviderTest (8).
• node --check e2e/xacml/xacml-test.spec.mjs and git diff a13a83c a639fdb --check passed.
• Docker Desktop was started successfully, but no configured OpenAM container was available; I did not run the full e2e suite or build a server WAR.

No other confirmed findings. The documented divergences and the explicitly deferred form-body issue are not being raised again. I recommend changes for the compressed-import regression.

[maximthomas]

Confirmed and fixed in bbdce70.

Restlet 2.4.4's Application adds an enabled DecoderService, whose inbound Decoder(context, true, false) decoded the request entity before importXACML read it. The branch handed getRawContentInputStream() to the parser.

• XacmlService.importXACML now reads Entity.newDecodedContentInputStream() and closes it once the import returns. An unsupported coding answers 400 Unsupported Content-Encoding: <coding>. Any other IOException while the stream is opened (for example Not in GZIP format) answers 400 with its message, as master's IOException branch did. A decoding failure later in the body still answers 400 XML cannot be parsed. through JAXB.
• XacmlServiceTest (now 25) adds importDecodesAGzipBody, importClosesTheDecodedBody, importWithAnUnsupportedContentEncodingAnswers400Xml and importWhoseGzipBodyIsNotGzipAnswers400Xml. The two tests that read the captured body now read it inside the stub, because the stream is closed afterwards. With the raw stream restored, the four new tests fail; without the close, importClosesTheDecodedBody fails. The module passes 616/616.
• New e2e test reads a gzip-compressed document (a dryrun import of a gzip-compressed export). Each WAR ran the spec twice on one container. Branch WAR with the fix: 23/0. Branch WAR before the fix: 22/1, failing only on this test (Expected 200, Received 400). Master WAR: 17/6, the same six known divergences, so the new test passes on master.

Not at parity, and now listed in the PR body and the upgrade note: CHF 3.2.2 decodes only gzip and deflate, and its DeflateDecoder reads deflate as raw DEFLATE (new Inflater(true)). RFC 9110 section 8.4.1.2 and Restlet use the zlib format. So a zlib deflate body answers 400, and Restlet's deflate-no-wrap, zip and identity answer 400 Unsupported Content-Encoding. Under the #1075 rule that CHF defects are fixed in commons, the DeflateDecoder fix is tracked in OpenIdentityPlatform/commons#385; XacmlService already decodes through CHF and will pick it up.

[vharseko]

Re-review of PR #1214 at head bbdce70.

The previous P2 finding is addressed. XacmlService now reads the import body through Entity.newDecodedContentInputStream() after checking MODIFY permission and closes the decoded stream in a finally block. Unsupported content encodings and I/O failures while opening the stream produce HTTP 400 XML responses. Failures during XML parsing still follow the existing EntitlementException path.

The four new unit tests cover gzip decoding, stream closure, an unsupported encoding, and a body that is not in its declared gzip format. The new e2e case compresses a real export, imports it with dryrun=true into the test sub-realm, checks the expected import step, and verifies that the policy was not persisted.

I reviewed the full PR discussion, the migration constraints in #1075, the related characterization work, and OpenIdentityPlatform/commons#385. The documented deflate/other-encoding differences and the previously deferred bare form-content-type issue are not being raised again.

Validation on this head:

• 43 selected unit tests passed on JDK 21: EndpointsTest (8), RestEndpointServletTest (2), XacmlServiceTest (25), and XacmlHttpRouteProviderTest (8), with no failures, errors, or skips. The Maven reactor build completed successfully.
• node --check e2e/xacml/xacml-test.spec.mjs and git diff a13a83c bbdce70 --check passed.
• I did not run the full-server e2e suite or repeat the author's mutation runs. CI was still pending when checked.

No further confirmed findings. I recommend approval of the code change; merge should wait for CI to pass.