Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-51971

StreamTaskListener#getCharset() may violate API and return null

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Minor Minor
    • core

      Current state:

      • API users may get NPE if they handle TaskListener according to the requirements

      Nonnull annotation has been introduced by jglick in https://github.com/jenkinsci/jenkins/commit/9ed6b01317424f3f441022b3a2f23fbbc5ae1543

      I see two options:

      • Relax requirements in TaskListener#getCharset() and allow null returns. It is a "breaking change" from API PoV (now we require API users to check for nulls)
      • Rework StreamTaskListener to disallow nulls and to default to something (UTF-8). May break some use-cases as well since now it defaults to the system's default charset

          [JENKINS-51971] StreamTaskListener#getCharset() may violate API and return null

          Oleg Nenashev created issue -
          Oleg Nenashev made changes -
          Description Original: * TaskListener#getCharset() always returns Nonnull according to the documentation
          * StreamTaskListener accepts null Charset in API and then may return null in getCharset()

          Once I added FindBugs annotations for StreamTaskListener in https://github.com/jenkinsci/jenkins/pull/3489 , FindBugs predictably failed
          New: * TaskListener#getCharset() always returns Nonnull according to the documentation: https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/util/StreamTaskListener.java
          * StreamTaskListener accepts null Charset in API and then may return null in getCharset()
          * Once I added FindBugs annotations for StreamTaskListener in https://github.com/jenkinsci/jenkins/pull/3489 , FindBugs predictably failed.
          * StreamTaskListener checks getCharset() for null in the code internally, so probably the annotation should be just changed to Nonnull. But it's formally incompatible change

          Nonnull annotation has been introduced by [~jglick] in https://github.com/jenkinsci/jenkins/commit/9ed6b01317424f3f441022b3a2f23fbbc5ae1543

          I see two options:
          * Relax requirements in TaskListener#getCharset() and allow null returns. It is a "breaking change" from API PoV (now we require API users to check for nulls)
          * Rework StreamTaskListener to disallow nulls and to default to something (UTF-8). May break some use-cases as well since now it defaults to the system's default charset
          Oleg Nenashev made changes -
          Description Original: * TaskListener#getCharset() always returns Nonnull according to the documentation: https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/util/StreamTaskListener.java
          * StreamTaskListener accepts null Charset in API and then may return null in getCharset()
          * Once I added FindBugs annotations for StreamTaskListener in https://github.com/jenkinsci/jenkins/pull/3489 , FindBugs predictably failed.
          * StreamTaskListener checks getCharset() for null in the code internally, so probably the annotation should be just changed to Nonnull. But it's formally incompatible change

          Nonnull annotation has been introduced by [~jglick] in https://github.com/jenkinsci/jenkins/commit/9ed6b01317424f3f441022b3a2f23fbbc5ae1543

          I see two options:
          * Relax requirements in TaskListener#getCharset() and allow null returns. It is a "breaking change" from API PoV (now we require API users to check for nulls)
          * Rework StreamTaskListener to disallow nulls and to default to something (UTF-8). May break some use-cases as well since now it defaults to the system's default charset
          New: * TaskListener#getCharset() always returns Nonnull according to the documentation: https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/util/StreamTaskListener.java
          * StreamTaskListener accepts null Charset in API and then may return null in getCharset()
          * Once I added FindBugs annotations for StreamTaskListener in https://github.com/jenkinsci/jenkins/pull/3489 , FindBugs predictably failed.
          * TaskListener checks getCharset() for null in the code internally, so probably the annotation should be just changed to Nonnull. But it's formally incompatible change

          Nonnull annotation has been introduced by [~jglick] in https://github.com/jenkinsci/jenkins/commit/9ed6b01317424f3f441022b3a2f23fbbc5ae1543

          I see two options:
          * Relax requirements in TaskListener#getCharset() and allow null returns. It is a "breaking change" from API PoV (now we require API users to check for nulls)
          * Rework StreamTaskListener to disallow nulls and to default to something (UTF-8). May break some use-cases as well since now it defaults to the system's default charset
          Oleg Nenashev made changes -
          Description Original: * TaskListener#getCharset() always returns Nonnull according to the documentation: https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/util/StreamTaskListener.java
          * StreamTaskListener accepts null Charset in API and then may return null in getCharset()
          * Once I added FindBugs annotations for StreamTaskListener in https://github.com/jenkinsci/jenkins/pull/3489 , FindBugs predictably failed.
          * TaskListener checks getCharset() for null in the code internally, so probably the annotation should be just changed to Nonnull. But it's formally incompatible change

          Nonnull annotation has been introduced by [~jglick] in https://github.com/jenkinsci/jenkins/commit/9ed6b01317424f3f441022b3a2f23fbbc5ae1543

          I see two options:
          * Relax requirements in TaskListener#getCharset() and allow null returns. It is a "breaking change" from API PoV (now we require API users to check for nulls)
          * Rework StreamTaskListener to disallow nulls and to default to something (UTF-8). May break some use-cases as well since now it defaults to the system's default charset
          New: * TaskListener#getCharset() always returns Nonnull according to the documentation: https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/util/StreamTaskListener.java
          * StreamTaskListener accepts null Charset in API and then may return null in getCharset()
          * Once I added FindBugs annotations for StreamTaskListener in https://github.com/jenkinsci/jenkins/pull/3489 , FindBugs predictably failed.
          * TaskListener checks getCharset() for null in the code internally, so probably the annotation should be just changed to Nonnull. But it's formally incompatible change

          Current state:

          * API users may get NPE if they handle TaskListener according to the requirements

          Nonnull annotation has been introduced by [~jglick] in https://github.com/jenkinsci/jenkins/commit/9ed6b01317424f3f441022b3a2f23fbbc5ae1543

          I see two options:
          * Relax requirements in TaskListener#getCharset() and allow null returns. It is a "breaking change" from API PoV (now we require API users to check for nulls)
          * Rework StreamTaskListener to disallow nulls and to default to something (UTF-8). May break some use-cases as well since now it defaults to the system's default charset
          Oleg Nenashev made changes -
          Status Original: Open [ 1 ] New: In Progress [ 3 ]
          Oleg Nenashev made changes -
          Resolution New: Fixed [ 1 ]
          Status Original: In Progress [ 3 ] New: Resolved [ 5 ]
          Oleg Nenashev made changes -
          Labels New: lts-candidate
          Oliver Gondža made changes -
          Labels Original: lts-candidate New: 2.121.2-fixed

            Unassigned Unassigned
            oleg_nenashev Oleg Nenashev
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: