MyFaces Core
  1. MyFaces Core
  2. MYFACES-3132

javax.faces.context.ResponseWriterWrapper implementation is overdone

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.6, 2.1.0
    • Component/s: None
    • Labels:
      None

      Description

      When using IceFaces an NPE is encountered when one of the lower-level Writer calls is made against its DomResponseWriter (specifically this was a result of HtmlRendererUtils.renderSelectOptions() calling writer.write(TABULATOR)). This is because java.io.Writer delegates all forms of #write() back to the abstract write(char[], int, int) variant. MyFaces' version of ResponseWriterWrapper implements write(int) directly, whereas the spec actually not does declare this method as being implemented here and thus allows the default implementation from Writer to delegate to the char[], int, int version. Since the MyFaces version calls getWrapped().write(int), an NPE is thrown that would be avoided if IceFaces were permitted to proceed through the call sequence as implicitly promised by the spec. True implementation of the spec requires deleting each of:

      • append(char)
      • append(CharSequence)
      • append(CharSequence, int, int)
      • write(char[])
      • write(int)
      • write(String)
      • write(String, int, int)

        Activity

        Matt Benson created issue -
        Leonardo Uribe made changes -
        Field Original Value New Value
        Attachment MYFACES-3132-1.patch [ 12478628 ]
        Hide
        Leonardo Uribe added a comment -

        The base javadoc of java.io.Writer says this:

        "... Abstract class for writing to character streams. The only methods that a subclass must implement are write(char[], int, int), flush(), and close(). Most subclasses, however, will override some of the methods defined here in order to provide higher efficiency, additional functionality, or both..."

        so all other methods are not necessary to implement for javax.faces.context.ResponseWriterWrapper class.

        Show
        Leonardo Uribe added a comment - The base javadoc of java.io.Writer says this: "... Abstract class for writing to character streams. The only methods that a subclass must implement are write(char[], int, int), flush(), and close(). Most subclasses, however, will override some of the methods defined here in order to provide higher efficiency, additional functionality, or both..." so all other methods are not necessary to implement for javax.faces.context.ResponseWriterWrapper class.
        Leonardo Uribe made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Assignee Leonardo Uribe [ lu4242 ]
        Fix Version/s 2.0.6 [ 12316397 ]
        Fix Version/s 2.1.0 [ 12315190 ]
        Resolution Fixed [ 1 ]
        Hide
        Matt Benson added a comment -

        I do feel it is important to distinguish the fact that MyFaces' implementation of the javax.faces APIs should exactly implement the specification, making the implemention of these methods a deficit rather than a benefit. This would likely almost never be the case with any "normal" situation. In any event, thanks for addressing this!

        Show
        Matt Benson added a comment - I do feel it is important to distinguish the fact that MyFaces' implementation of the javax.faces APIs should exactly implement the specification, making the implemention of these methods a deficit rather than a benefit. This would likely almost never be the case with any "normal" situation. In any event, thanks for addressing this!
        Hide
        Matt Benson added a comment -

        I think the problem that was caused by this has indeed been solved. This, however, seems to be only the tip of the iceberg wrt making Icefaces compatible with MyFaces, but we shall see.

        Thanks!

        Show
        Matt Benson added a comment - I think the problem that was caused by this has indeed been solved. This, however, seems to be only the tip of the iceberg wrt making Icefaces compatible with MyFaces, but we shall see. Thanks!
        Leonardo Uribe made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Leonardo Uribe
            Reporter:
            Matt Benson
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development