SmartSprites
  1. SmartSprites
  2. SMARTSPRITES-74

Negative values of sprite-margin cause java.lang.ArrayIndexOutOfBoundsException

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.2.6
    • Fix Version/s: 0.2.7
    • Labels:
      None

      Description

      If in the css file we put a negative value to sprite-margin-XXXX it throws a ArrayIndexOutOfBoundsException.

      I tried to fix it in source code. My changes affects the 3 files:

      org.carrot2.labs.smartsprites.message.Message
      ----------------------------------------------
      -added in MessageType :

      CANNOT_PARSE_NEGATIVE_MARGIN_VALUE("Cannot parse negative margin value: %s. Using '0'"),

      org.carrot2.labs.smartsprites.SpriteImageDirective
      -------------------------------------------------------
      -added a parameter "MessageLog messageLog" in SpriteImageDirective function. So the functions is like:

      public SpriteImageDirective(String id, String imageUrl, SpriteImageLayout layout,
      SpriteImageFormat format, Ie6Mode ie6Mode, Color matteColor, SpriteUidType uidType, MessageLog messageLog)

      { this(id, imageUrl, layout, format, ie6Mode, matteColor, uidType, new SpriteLayoutProperties(layout, messageLog)); }

      org.carrot2.labs.smartsprites.SpriteLayoutProperties
      -----------------------------------------------------
      -changes in the 2 constructors:

      public SpriteLayoutProperties(SpriteAlignment alignment, int marginLeft,
      int marginRight, int marginTop, int marginBottom, MessageLog messageLog)
      {
      this.alignment = alignment;
      if(marginLeft<0)

      { messageLog.warning(MessageType.CANNOT_PARSE_NEGATIVE_MARGIN_VALUE, marginLeft); this.marginLeft = 0; }

      else

      { this.marginLeft = marginLeft; }

      if(marginRight<0)

      { messageLog.warning(MessageType.CANNOT_PARSE_NEGATIVE_MARGIN_VALUE, marginRight); this.marginRight = 0; }

      else

      { this.marginRight = marginRight; }

      if(marginTop<0)

      { messageLog.warning(MessageType.CANNOT_PARSE_NEGATIVE_MARGIN_VALUE, marginTop); this.marginTop = 0; }

      else

      { this.marginTop = marginTop; }

      if(marginBottom<0)

      { messageLog.warning(MessageType.CANNOT_PARSE_NEGATIVE_MARGIN_VALUE, marginBottom); this.marginBottom = 0; }

      else

      { this.marginBottom = marginBottom; }

      }

      /**

      • Creates an instance with default values.
        */
        SpriteLayoutProperties(SpriteImageLayout layout, MessageLog messageLog) { this(getDefaultAlignment(layout), 0, 0, 0, 0, messageLog); }

      -added the parameter in the call in 'parse' method (line 129):

      return parse(directiveString, spriteImageLayout, new SpriteLayoutProperties(
      spriteImageLayout, messageCollector), messageCollector);

      -added the parameter in the call in 'parse' method (line 181):

      return new SpriteLayoutProperties(alignment, marginLeft, marginRight, marginTop,
      marginBottom);

      -Change the try/catch content in getMargin metod (line 246):

      try
      {
      int value = Integer.parseInt(marginValue);
      if (value<0)

      { messageLog.warning(MessageType.CANNOT_PARSE_NEGATIVE_MARGIN_VALUE, rawMarginValue); return 0; }

      else

      { return value; }

      }
      catch (final NumberFormatException e)

      { messageLog.warning(MessageType.CANNOT_PARSE_MARGIN_VALUE, rawMarginValue); return 0; }

        Activity

        alejandro anadon created issue -
        Hide
        alejandro anadon added a comment -

        Here are the files

        Show
        alejandro anadon added a comment - Here are the files
        alejandro anadon made changes -
        Field Original Value New Value
        Attachment Message.java [ 10300 ]
        Attachment SpriteImageDirective.java [ 10301 ]
        Attachment SpriteLayoutProperties.java [ 10302 ]
        Hide
        Stanisław Osiński added a comment -

        Hi Alejandro,

        Thanks for the patch. Later today I'll review it, commit to the repository and reference on the website.

        Thanks!

        S.

        Show
        Stanisław Osiński added a comment - Hi Alejandro, Thanks for the patch. Later today I'll review it, commit to the repository and reference on the website. Thanks! S.
        Stanisław Osiński made changes -
        Fix Version/s 0.2.7 [ 10183 ]
        Hide
        Stanisław Osiński added a comment -

        Hi Alejandro,

        I've been trying to write a unit test for this bug, but I don't seem to be able to find the combination of images and margins that throw the exception. Would you be able to provide the design that did not work?

        Thanks!

        S.

        Show
        Stanisław Osiński added a comment - Hi Alejandro, I've been trying to write a unit test for this bug, but I don't seem to be able to find the combination of images and margins that throw the exception. Would you be able to provide the design that did not work? Thanks! S.
        alejandro anadon made changes -
        Attachment smart-sprites-examples.zip [ 10310 ]
        Attachment ENFSmartsprites.java [ 10311 ]
        alejandro anadon made changes -
        Attachment ENFSmartsprites.java [ 10312 ]
        Attachment smart-sprites-examples.zip [ 10313 ]
        Hide
        alejandro anadon added a comment -

        Hi Stanisław,

        I attached two files (well... I made a mistake and attached files two times... so I attached 4 files but 2 files are the same to the other 2 and I do not know how to delete the duplicated files).

        One is:

        ENFSmartsprites.java This Class is only foe execute a main file and test smart-sprites (I added in case you need it)

        The other is:

        smart-sprites-examples.zip it has the resources within I run the ENFSmartsprites it throws the exception.
        Th resorces are the same that you you put in a example (I don't remember where I saw it) but setting a negatice valute in the C:\smart-sprites-examples\p2\css\style.css file in:

        #logo {

        ...... /* ..... : sprite-margin-right: -16px; */
        ....

        Show
        alejandro anadon added a comment - Hi Stanisław, I attached two files (well... I made a mistake and attached files two times... so I attached 4 files but 2 files are the same to the other 2 and I do not know how to delete the duplicated files). One is: ENFSmartsprites.java This Class is only foe execute a main file and test smart-sprites (I added in case you need it) The other is: smart-sprites-examples.zip it has the resources within I run the ENFSmartsprites it throws the exception. Th resorces are the same that you you put in a example (I don't remember where I saw it) but setting a negatice valute in the C:\smart-sprites-examples\p2\css\style.css file in: #logo { ...... /* ..... : sprite-margin-right: -16px; */ ....
        Hide
        Stanisław Osiński added a comment -

        Thanks, Alejandro! I'll convert the example into a JUnit test and commit to the repository.

        Show
        Stanisław Osiński added a comment - Thanks, Alejandro! I'll convert the example into a JUnit test and commit to the repository.
        Stanisław Osiński logged work - 18/Jan/11 7:53 PM
        • Time Spent:
          19m
           
          Fix, unit tests.
        Hide
        Stanisław Osiński added a comment -

        Fixed / committed in trunk.

        Show
        Stanisław Osiński added a comment - Fixed / committed in trunk.
        Stanisław Osiński made changes -
        Original Estimate 0h [ 0 ]
        Remaining Estimate 0h [ 0 ]
        Stanisław Osiński made changes -
        Time Spent 19m [ 1140 ]
        Worklog Id 10312 [ 10312 ]
        Stanisław Osiński made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Stanisław Osiński made changes -
        Summary Negative values to sprite-margin-XXXX makes a "Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: Coordinate out of bounds!" Negative values of sprite-margin cause java.lang.ArrayIndexOutOfBoundsException

          People

          • Assignee:
            Stanisław Osiński
            Reporter:
            alejandro anadon
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 0h
              0h
              Remaining:
              Remaining Estimate - 0h
              0h
              Logged:
              Time Spent - 19m
              19m

                Development