[Pki-devel] [PATCH] TRAC Ticket #868 - REST API get certs links missing segment [20140307]
Ade Lee
alee at redhat.com
Mon Mar 17 13:29:27 UTC 2014
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%28java.lang.Class,%20java.lang.String%29
> >
> > 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 at redhat.com
> > > https://www.redhat.com/mailman/listinfo/pki-devel
> >
>
More information about the Pki-devel
mailing list