<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <br>
    <div class="moz-cite-prefix">On 1/15/14 9:37 AM, Ian Young wrote:<br>
    </div>
    <blockquote
      cite="mid:BA3D57B7-4360-4F12-8F7B-A9F3DEF92FAD@iay.org.uk"
      type="cite">
      <pre wrap="">While reviewing Rod's list of orphaned or near-orphaned classes in java-support, I took a look at the IndexingObjectStore class. The intention of this, looking at its Javadoc, is to compress the in-memory storage of large objects which are repeated by using the objects' hash as an index into a reference-counted storage system. I think the prototypical application is X.509 certs in metadata.</pre>
    </blockquote>
    <br>
    Yes, this was a Chad-introduced thing from way back.&nbsp; He put it in
    v2 a long time ago (Dec 2008) after some profiling of the IdP showed
    that we were storing a lot of cert data in memory.&nbsp;&nbsp;&nbsp; So it's been
    in the IdP for 5+ years at this point (unfortunately).&nbsp; I had never
    really looked at it in detail and had mostly forgotten that it was
    even there... <br>
    <br>
    <br>
    <blockquote
      cite="mid:BA3D57B7-4360-4F12-8F7B-A9F3DEF92FAD@iay.org.uk"
      type="cite">
      <pre wrap="">


In other words, if there are two objects which have different hashes, they had better have identical functionality or IndexedObjectStore will Do The Wrong Thing.

That seems to be a dangerous assumption for a class to be making (and it is an assumption, as far as I can tell the code doesn't validate it). I can't see how one can guarantee that the assumption will hold for X509-certs-as-strings, which seems to be the OpenSAML use case. Of course, for most real data sets, the assumption will not be violated, but I am more than a little concerned that things might go wrong in hard to understand ways here.
</pre>
    </blockquote>
    <br>
    <br>
    It's a dangerous and invalid assumption, I agree.&nbsp; OpenSAML can't
    guarantee this assumption.&nbsp; And IMHO we would never figure out this
    sort of hash collision in a real world scenario, and worse, even if
    we could, the only fix would be to ask someone to change their cert
    in some trivial way to make the hash different (which is
    unacceptable).<br>
    <br>
    IMHO program correctness always trumps optimization.&nbsp; If we can't
    devise a safe way to do this compression/memory optimization, then I
    think we need to pull it and just revert cert and CRL impl classes
    to just storing their strings like standard XMLObjects.<br>
  </body>
</html>