MyFaces Core
  1. MyFaces Core
  2. MYFACES-1681

UIComponent.getAttributes() too restrictive

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.5, 1.2.0
    • Fix Version/s: 1.2.2
    • Component/s: JSR-252
    • Labels:
      None
    • Environment:
      SE 1.6.0_02
      GlassFish V2 admingui

      Description

      The Map returned by UIComponent.getAttributes() is too restrictive with respect
      to null values.

      MyFaces will always throw an NPE for a null value. Granted, the following is
      in the javadocs:

      • Any attempt to add a null key or value must throw a NullPointerException.

      However, the following is also in the same section:

      • put() - If the property is writeable, call the setter method to set the corresponding value (unwrapping primitive values in their
        corresponding wrapper classes). If the property is not writeable, or an attempt is made to set a property of primitive type to null, throw
        IllegalArgumentException.

      Notice the comment about setting a primitive property to null. This implies that a null value for this case
      is legal. The RI will only throw the NPE against a null value if there isn't an associated property.

      1. MYFACES-1681.patch
        3 kB
        Matthias Weßendorf

        Activity

        Hide
        Ryan Lubke added a comment -

        Also, I plan on logging a request against the spec to clarifiy the javadocs.

        Show
        Ryan Lubke added a comment - Also, I plan on logging a request against the spec to clarifiy the javadocs.
        Hide
        Matthias Weßendorf added a comment -

        provided patch / test-case. waiting for TCK...

        Show
        Matthias Weßendorf added a comment - provided patch / test-case. waiting for TCK...
        Hide
        Matthias Weßendorf added a comment -

        Hey Ryan,

        thanks for pointing this out.
        This is 100% same code as in MyFaces 1.1.x.
        So should we address this in JSF 1.1 as well ?

        Show
        Matthias Weßendorf added a comment - Hey Ryan, thanks for pointing this out. This is 100% same code as in MyFaces 1.1.x. So should we address this in JSF 1.1 as well ?
        Hide
        Matthias Weßendorf added a comment -

        true for 1.1.x as well

        Show
        Matthias Weßendorf added a comment - true for 1.1.x as well
        Hide
        Martin Marinschek added a comment -

        We did this as the TCK failed, what I remember - it's only a faint memory though.

        regards,

        Martin

        Show
        Martin Marinschek added a comment - We did this as the TCK failed, what I remember - it's only a faint memory though. regards, Martin
        Hide
        Ryan Lubke added a comment -

        Right now both implementations pass the TCK, but this is only because of how the test was written.
        I'm pretty sure if this change is made, the TCK will continue to pass.

        Show
        Ryan Lubke added a comment - Right now both implementations pass the TCK, but this is only because of how the test was written. I'm pretty sure if this change is made, the TCK will continue to pass.

          People

          • Assignee:
            Matthias Weßendorf
            Reporter:
            Ryan Lubke
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development