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

JNLP slaves can fail to correctly negotiate a transport

XMLWordPrintable

      The changes in PR 41 introduced a regression whereby the read-ahead buffered input stream gets thrown away after the protocol has been detected but before the protocol negotiation starts.

      The result of this is that depending on random timing factors, the capability and mode information that has been sent to the remote side may get lost and one side will infer a capability of 0 while the other side believes the agreed capability to be more.

      When the remote side is assuming chunking, the connection will typically fail immediately with an error such as:

      INFO: Protocol failed to establish channel
java.io.StreamCorruptedException: invalid stream header: 0A6CACED
    at java.io.ObjectInputStream.readStreamHeader(ObjectInputStream.java:806)
    at java.io.ObjectInputStream.<init>(ObjectInputStream.java:299)
    at hudson.remoting.ObjectInputStreamEx.<init>(ObjectInputStreamEx.java:48)
    at hudson.remoting.ChannelBuilder.makeTransport(ChannelBuilder.java:430)
    at hudson.remoting.ChannelBuilder.negotiate(ChannelBuilder.java:389)
    at hudson.remoting.ChannelBuilder.build(ChannelBuilder.java:310)
    at org.jenkinsci.remoting.engine.JnlpProtocol2.buildChannel(JnlpProtocol2.java:93)
    at org.jenkinsci.remoting.engine.JnlpProtocol.establishChannel(JnlpProtocol.java:79)
    at hudson.remoting.Engine.run(Engine.java:245)

      But if the remote side is not assuming chunking then more subtle remoting issues could arise (e.g. if the remote slave is running an older pre-chunking slave.jar and connecting to a newer Jenkins... not that you should be doing that, but some people may... this reason is why I argue the issue is "Critical")

          [JENKINS-31718] JNLP slaves can fail to correctly negotiate a transport

          Stephen Connolly created issue -

          SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Stephen Connolly
          Path:
          src/main/java/hudson/remoting/Capability.java
          src/main/java/hudson/remoting/ChannelBuilder.java
          src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol.java
          src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java
          src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java
          src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java
          src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol1Test.java
          src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol2Test.java
          src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol3Test.java
          src/test/java/org/jenkinsci/remoting/engine/JnlpProtocolTest.java
          http://jenkins-ci.org/commit/remoting/116315728249eb392b9af0de0be5850959536c03
          Log:
          [FIXED JENKINS-31718] Preserve the BufferedInputStream when building the transport

          • For now I do not have the energy to fix the tests to not need the 'null' backdoor.
          • Most likely some non-mock based tests would be better and likely would have caught this regression sooner.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: src/main/java/hudson/remoting/Capability.java src/main/java/hudson/remoting/ChannelBuilder.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol1Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol2Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol3Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocolTest.java http://jenkins-ci.org/commit/remoting/116315728249eb392b9af0de0be5850959536c03 Log: [FIXED JENKINS-31718] Preserve the BufferedInputStream when building the transport For now I do not have the energy to fix the tests to not need the 'null' backdoor. Most likely some non-mock based tests would be better and likely would have caught this regression sooner.
          SCM/JIRA link daemon made changes -
          Resolution New: Fixed [ 1 ]
          Status Original: Open [ 1 ] New: Resolved [ 5 ]

          SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Stephen Connolly
          Path:
          src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java
          src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java
          src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java
          src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol1Test.java
          src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol2Test.java
          src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol3Test.java
          src/test/java/org/jenkinsci/remoting/engine/JnlpProtocolTest.java
          http://jenkins-ci.org/commit/remoting/015e280ddc20b858d810349a6243abba6741044b
          Log:
          JENKINS-31718 Fix the tests

          • The tests no longer pass in a null buffered input stream and thus we can remove the null handling from the protocols
          • Unsure whether the buffer may affect JnlpProtocol3 performance, but leaving it there to highlight the potential stream corruption if the buffer gets thrown away.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol1Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol2Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol3Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocolTest.java http://jenkins-ci.org/commit/remoting/015e280ddc20b858d810349a6243abba6741044b Log: JENKINS-31718 Fix the tests The tests no longer pass in a null buffered input stream and thus we can remove the null handling from the protocols Unsure whether the buffer may affect JnlpProtocol3 performance, but leaving it there to highlight the potential stream corruption if the buffer gets thrown away.
          Jesse Glick made changes -
          Resolution Original: Fixed [ 1 ]
          Status Original: Resolved [ 5 ] New: Reopened [ 4 ]
          Jesse Glick made changes -
          Status Original: Reopened [ 4 ] New: Open [ 1 ]
          Jesse Glick made changes -
          Status Original: Open [ 1 ] New: In Progress [ 3 ]
          Daniel Beck made changes -
          Link New: This issue is duplicated by JENKINS-31715 [ JENKINS-31715 ]

          Jesse Glick added a comment -

          Avoid using the FIXED prefix in commits which

          • Are in an origin branch. The JIRA link daemon does not check that they are ancestors of master.
          • Are in a component repository. The FIXED commit should only be in the jenkins repository when integrating a component release with the fix.

          Jesse Glick added a comment - Avoid using the FIXED prefix in commits which Are in an origin branch. The JIRA link daemon does not check that they are ancestors of master . Are in a component repository. The FIXED commit should only be in the jenkins repository when integrating a component release with the fix.
          Jesse Glick made changes -
          Remote Link New: This issue links to "PR 65 (Web Link)" [ 13551 ]

            stephenconnolly Stephen Connolly
            stephenconnolly Stephen Connolly
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: