On Fri, 2014-03-14 at 19:10 -0700, Matthew Harmsen wrote:

On 03/13/14 11:38, Ade Lee wrote:

...

This patch doesn't future proof against other possible URL changes. This bug basically arose because we changed the annotation for CertsResource from "/certs" to "". Ade,

While I understand your concern about not "future proofing" against other URL changes, I am confused by your statement that '... we changed the annotation for CertsResource from "/certs" to "".'

All CertResource.java (presuming "CertsResource.java" is a typo?) files currently contain "/certs" as a part of their annotations: * DOGTAG_10_0_BRANCH (Fedora 19 - this ticket): * ./base/common/src/com/netscape/certsrv/cert/CertResource.java * all annotations utilize "certs/{id}", not just "{id}" * ./base/server/cms/src/com/netscape/cms/servlet/cert/CertService.java * currently specifies "{id}" instead of "certs/{id}" which yields the 404 error: * URI uri = uriInfo.getBaseUriBuilder().path(CertResource.class).path("{id}").build(certId.toHexString()); * URI uri = uriInfo.getBaseUriBuilder().path(CertResource.class).path("{id}").build(id.toHexString());

Matt, my point is that the above code used to work because the top-level annotation in CertResource.java used to be "certs". At that time, then, the annotation for getCert() would have been "{id}", and the above formulation in CertService.java would have resulted in a correct result. It was however, not the best way of formulating the result. In fact, we see that it broke as soon as we modified the top level annotation from "certs" to "", and instead moved the annotation into the individual method. A better way to do this would have been to rather refer to the annotation of the specific method directly, rather than trying to build it from the top annotation - something like this: URI uri = uriInfo.getBaseUriBuilder().path(CertResource.class, "getCert").build(certId.toHexString()); All I was trying to show in the pastebin was an example of building the URI by referencing the specific method in the resource. Ade

* DOGTAG_10_1_BRANCH (Fedora 20) * ./base/common/src/com/netscape/certsrv/cert/CertResource.java * all annotations utilize "certs/{id}", not just "{id}" * ./base/server/cms/src/com/netscape/cms/servlet/cert/CertService.java * currently specifies "{id}" instead of "certs/{id}" which yields the 404 error: * URI uri = uriInfo.getBaseUriBuilder().path(CertResource.class).path("{id}").build(certId.toHexString()); * URI uri = uriInfo.getBaseUriBuilder().path(CertResource.class).path("{id}").build(id.toHexString()); * master (Fedora 21+) * ./base/common/src/com/netscape/certsrv/cert/CertResource.java * all annotations utilize "certs/{id}", not just "{id}" * ./base/ca/src/org/dogtagpki/server/ca/rest/CertService.java * currently specifies "{id}" instead of "certs/{id}" which yields the 404 error: * URI uri = uriInfo.getBaseUriBuilder().path(CertResource.class).path("{id}").build(certId.toHexString()); * URI uri = uriInfo.getBaseUriBuilder().path(CertResource.class).path("{id}").build(id.toHexString());

Are you requesting that we change CertResource.java to remove all of the "/certs" prefixes from the annotations instead of changing the CertService.java implementations?

Or, based upon your pastebin:

uriInfo.getBaseUriBuilder() .path(ShowsResource.class, "getShow") .build(show.getId());

@Path("shows") public class ShowsResource {

@GET @Path("{showId}") public String getShow ( @PathParam("showId") long showId) { return "HAHAHAHA"; } }

Generates: "context-root/45" Instead of "context-root/shows/45" are you are trying to introduce a new class (e. g. - ShowsResource.java) which extends all of the other Resource classes so that implementation Service classes such as CertService.java will call ShowsResource.class instead of their CertResource.class directly?

I presume that this interim class would provide an unchanging interface to all of the underlying Resource classes (but perhaps its annotations might need to change whenever an underlying Resource changes)?

To fix the immediate bug, could we simply check in my change to the various branches and create a ticket for this work? Presuming that the other two tickets ( PKI TRAC Ticket #728 - null pointer exception thrown when processing 501 from dogtag 9-> 10 instance and PKI TRAC Ticket #802 - Tomcat running as root) do not need to be fixed at this time, this would allow us to proceed with getting the Dogtag 10.0.7 builds finalized so that I can move on to the more pressing Dogtag 10.1.1 issue.

Thanks, -- Matt

...

I think you should be rather mapping to the relevant GET method instead using:

http://docs.oracle.com/javaee/6/api/javax/ws/rs/core/UriBuilder.html#path%28...

either: public abstract UriBuilder path(java.lang.Class resource, java.lang.String method)

or: public abstract UriBuilder path(java.lang.reflect.Method method)

In fact, we might want to open a ticket to confirm that all the rest of the URLs are likewise future-proofed against changes.

Ade

On Fri, 2014-03-07 at 18:48 -0800, Matthew Harmsen wrote:

...

Please review the following patch which addresses: * PKI TRAC Ticket #868 - REST API get certs links missing segment

This patch has been tested on the DOGTAG_10_0_BRANCH as used on Fedora 19:

Prior to the patch, the following URL:

* https://fedora19.example.com:8443/ca/rest/certs

produces an XML page which contains XML such as:

* <Link href="https://fedora19.example.com:8443/ca/rest/0x1" rel="self"/>

which produces an 'HTTP Status 404' page.

After the patch has been applied, the same URL produces an XML page which contains XML such as:

* <Link href="https://fedora19.example.com:8443/ca/rest/certs/0x1" rel="self"/>

which corresponds to a valid URL.

NOTE: This patch needs to be applied to the DOGTAG_10_0_BRANCH (Fedora 19), the DOGTAG_10_1_BRANCH (Fedora 20), and the master (Fedora 21+). This patch may also need to be applied to the IPA_V3_RHEL_7_ERRATA_BRANCH.

_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel