<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">Hello, Anton.<br>
      The fix looks good. <br>
      Thanks.<br>
      <br>
      On 16.04.2014 18:32, Andrew Brygin wrote:<br>
    </div>
    <blockquote cite="mid:534E9470.3070901@oracle.com" type="cite">
      <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
      <div class="moz-cite-prefix">Hello Anton,<br>
        <br>
         the fix looks fine to me.<br>
        <br>
        Thanks,<br>
        Andrew<br>
        <br>
        On 4/16/2014 6:30 PM, anton nashatyrev wrote:<br>
      </div>
      <blockquote cite="mid:534E93EA.9020509@oracle.com" type="cite">
        <meta content="text/html; charset=UTF-8"
          http-equiv="Content-Type">
        Hello Andrew, <br>
        <br>
            you are right, the fix may be less relaxing but still
        conforming: <br>
            if the height == 1, but the minY - sampleModelTranslateY
        > 0 the data buffer should still contain at least one
        scanline. <br>
            Below is the updated fix. I've also added explicit check for
        implicit (minY - sampleModelTranslateY >= 0) constraint.<br>
        <br>
            <a moz-do-not-send="true"
          href="http://cr.openjdk.java.net/%7Eanashaty/8038000/webrev.02/">http://cr.openjdk.java.net/%7Eanashaty/8038000/webrev.02/</a><br>
        <br>
        Thank you!<br>
        Anton.<br>
        <br>
        <div class="moz-cite-prefix">On 11.04.2014 15:06, Andrew Brygin
          wrote:<br>
        </div>
        <blockquote cite="mid:5347CCA4.4090907@oracle.com" type="cite">
          <meta content="text/html; charset=UTF-8"
            http-equiv="Content-Type">
          <div class="moz-cite-prefix">Hello Anton,<br>
            <br>
             we probably should use '(minY + height) > 1' as a
            condition for the <br>
             scanstride test, should not we?<br>
             Otherwise, we are not taking into account the case when
            minY is<br>
             greater than 0, and too big scanstride may be potentially
            dangerous.<br>
            <br>
            Thanks,<br>
            Andrew <br>
            <br>
            <br>
            On 4/10/2014 11:26 PM, anton nashatyrev wrote:<br>
          </div>
          <blockquote cite="mid:5346F064.9060600@oracle.com" type="cite">
            <meta content="text/html; charset=UTF-8"
              http-equiv="Content-Type">
            Hello,<br>
            <br>
                could you please review a slightly update fix version
            (the regression test upgraded) <br>
            fix: <a moz-do-not-send="true"
              href="http://cr.openjdk.java.net/%7Eanashaty/8038000/webrev.01/">http://cr.openjdk.java.net/%7Eanashaty/8038000/webrev.01/</a><br>
            bug: <a moz-do-not-send="true"
              href="https://bugs.openjdk.java.net/browse/JDK-8038000">https://bugs.openjdk.java.net/browse/JDK-8038000




            </a><br>
            <br>
            Jim, thanks for your in-depth analysis, the validation
            indeed doesn't look ideal. However my fix addresses the
            concrete regression which was introduced by these
            validations, so I'm leaving the fix as is. <br>
            <br>
            Thank you!<br>
            Anton.<br>
            <br>
            <div class="moz-cite-prefix">On 01.04.2014 19:39, anton
              nashatyrev wrote:<br>
            </div>
            <blockquote cite="mid:533ADD9F.50501@oracle.com" type="cite">Hello



              Jim, <br>
              <br>
              On 28.03.2014 3:25, Jim Graham wrote: <br>
              <blockquote type="cite">Hi Anton, <br>
                <br>
                A lot of those tests seem out of whack in that they test
                related conditions, but not the exact condition itself. 
                What we really want is for every index of the form: <br>
                <br>
                offset + y * scanlineStride + x + {0 ->
                numcomponents-1} => [0, buf.length-1] <br>
                <br>
                to be in the array for all valid x,y.  There are a lot
                of tests there that are sufficient to prove this, but
                are overly restrictive in that they reject a bunch of
                cases.  The fix you propose only fixes this for a case
                where h=1, but what about: <br>
                <br>
                    w = 10 <br>
                    h = 2 <br>
                    numcomponents = 1 <br>
                    scanlineStride = 1000 <br>
                    buffer.length = 1010 <br>
                <br>
                The buffer contains the data for all possible pixel
                fetches, but since it isn't 2000 in length, we reject
                it. <br>
              </blockquote>
              <br>
              My fix actually relaxes the condition, and the case above
              is not rejected: <br>
              (height > 1 && scanlineStride > data.length)
              is FALSE here and hence the exception is not thrown <br>
              <br>
              <blockquote type="cite"> <br>
                Also, we restrict a bunch of parameters to "MAX_INT /
                some other factor" because we blindly multiply things
                out and don't want to deal with overflow, but a better
                "pixel array" test would use division (I think we do
                this in our native code): <br>
                <br>
                    buf.length / w <= h <br>
                <br>
                except that you need to deal with offset and
                scanlineStride for h-1 lines and then w for the last
                one, so this gets complicate, but you have: <br>
                <br>
                    ((buf.length - offset - w) / (h-1)) <
                scanlineStride <br>
                <br>
                but then you have to special case h=1 to avoid the
                divide by 0 so you get: <br>
                <br>
                    if (w <= 0 || h <= 0 || scanlineStride < 0
                || offset < 0) exception; <br>
                    if (offset >= buf.length) exception; <br>
                    int len = buf.length - offset; // known positive <br>
                    if (len < w) exception; <br>
                    if (h > 1) { <br>
                         if (((len - w) / (h - 1)) < scanlineStride)
                exception; <br>
                    } <br>
                <br>
                Note that the test for (len < w) is done in all cases
                because it prevents the calculation in the "h>1" case
                from having a negative numerator, which would make the
                test invalid.  These tests are also valid for scan=0 for
                the rare case where someone wants every row of an image
                to contain the same data (we may use a case like that in
                the GIF loading code that needs to replicate incoming
                data for interlaced GIFs).  It doesn't go so far as to
                allow scan=-1 or similar cases where the user wants to
                play games with aliasing parts of rows in a backwards
                cascade. <br>
              </blockquote>
              <br>
              There are a lot of checks in the *Raster.verify() methods.
              I'm not 100% confident but they look pretty equivalent to
              the algorithm you have described (BTW the 0 scanline is
              also acceptable). Anyways that was a security fix some
              time ago when some of those validations have been added
              and I'm not sure we would like to perform some major
              refactorings here unless any incompatibilities are found.
              <br>
              <br>
              Thank you! <br>
              Anton. <br>
              <br>
              <blockquote type="cite"> <br>
                            ...jim <br>
                <br>
                On 3/26/14 10:35 AM, anton nashatyrev wrote: <br>
                <blockquote type="cite">Hello, <br>
                       could you please review the following fix: <br>
                  <br>
                  fix: <a moz-do-not-send="true"
                    class="moz-txt-link-freetext"
                    href="http://cr.openjdk.java.net/%7Eanashaty/8038000/webrev.00/">http://cr.openjdk.java.net/~anashaty/8038000/webrev.00/</a>
                  <br>
                  <a moz-do-not-send="true"
                    class="moz-txt-link-rfc2396E"
                    href="http://cr.openjdk.java.net/%7Eanashaty/8038000/webrev.00/"><http://cr.openjdk.java.net/%7Eanashaty/8038000/webrev.00/></a>
                  <br>
                  bug: <a moz-do-not-send="true"
                    class="moz-txt-link-freetext"
                    href="https://bugs.openjdk.java.net/browse/JDK-8038000">https://bugs.openjdk.java.net/browse/JDK-8038000</a>
                  <br>
                  <br>
                  The last row in the Raster shouldn't be necessary of
                  the scanlineStride <br>
                  length and thus the Raster with height 1 might have a
                  buffer smaller <br>
                  than a scanlineStride. <br>
                  <br>
                  Thanks! <br>
                  Anton. <br>
                </blockquote>
              </blockquote>
              <br>
            </blockquote>
            <br>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
    <br>
    <pre class="moz-signature" cols="72">-- 
Best regards, Sergey. </pre>
  </body>
</html>