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

JNLP slaves can fail to correctly negotiate a transport

      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

          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 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.

          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/17aa60dd6c84499962a7117027da3e73e392ce46
          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/17aa60dd6c84499962a7117027da3e73e392ce46 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.

          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/7a50cd2e8e8d40b243eab5584e5ae44d48215b2d
          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/7a50cd2e8e8d40b243eab5584e5ae44d48215b2d 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 added a comment -

          Not fixed until integrated into core.

          Jesse Glick added a comment - Not fixed until integrated into core.

          Code changed in jenkins
          User: Jesse Glick
          Path:
          pom.xml
          http://jenkins-ci.org/commit/jenkins/e6d8500dd8e9b157ae8c54aec6fd42286e88301a
          Log:
          [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: pom.xml http://jenkins-ci.org/commit/jenkins/e6d8500dd8e9b157ae8c54aec6fd42286e88301a Log: [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections.

          Code changed in jenkins
          User: Kohsuke Kawaguchi
          Path:
          pom.xml
          http://jenkins-ci.org/commit/jenkins/940796f04fc43aed750aff970bc4fad4793f068f
          Log:
          Merge pull request #1927 from jglick/remoting-JENKINS-31718

          JENKINS-31718 Integrated a new version of Remoting

          Compare: https://github.com/jenkinsci/jenkins/compare/e930da45ff4c...940796f04fc4

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Kohsuke Kawaguchi Path: pom.xml http://jenkins-ci.org/commit/jenkins/940796f04fc43aed750aff970bc4fad4793f068f Log: Merge pull request #1927 from jglick/remoting- JENKINS-31718 JENKINS-31718 Integrated a new version of Remoting Compare: https://github.com/jenkinsci/jenkins/compare/e930da45ff4c...940796f04fc4

          Code changed in jenkins
          User: Jesse Glick
          Path:
          pom.xml
          http://jenkins-ci.org/commit/jenkins/9e321a4a0eefde421709647f0f15cb7bc8da61d1
          Log:
          [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections.

          (cherry picked from commit e6d8500dd8e9b157ae8c54aec6fd42286e88301a)

          In the project meeting today we decided to pull this in.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: pom.xml http://jenkins-ci.org/commit/jenkins/9e321a4a0eefde421709647f0f15cb7bc8da61d1 Log: [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections. (cherry picked from commit e6d8500dd8e9b157ae8c54aec6fd42286e88301a) In the project meeting today we decided to pull this in.

          dogfood added a comment -

          Integrated in jenkins_main_trunk #4373
          [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections. (Revision e6d8500dd8e9b157ae8c54aec6fd42286e88301a)
          [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections. (Revision 9e321a4a0eefde421709647f0f15cb7bc8da61d1)

          Result = SUCCESS
          jesse glick : e6d8500dd8e9b157ae8c54aec6fd42286e88301a
          Files :

          • pom.xml

          kohsuke : 9e321a4a0eefde421709647f0f15cb7bc8da61d1
          Files :

          • pom.xml

          dogfood added a comment - Integrated in jenkins_main_trunk #4373 [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections. (Revision e6d8500dd8e9b157ae8c54aec6fd42286e88301a) [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections. (Revision 9e321a4a0eefde421709647f0f15cb7bc8da61d1) Result = SUCCESS jesse glick : e6d8500dd8e9b157ae8c54aec6fd42286e88301a Files : pom.xml kohsuke : 9e321a4a0eefde421709647f0f15cb7bc8da61d1 Files : pom.xml

          dogfood added a comment -

          Integrated in jenkins_2.0 #5
          [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix (Revision e6d8500dd8e9b157ae8c54aec6fd42286e88301a)
          [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix (Revision 9e321a4a0eefde421709647f0f15cb7bc8da61d1)

          Result = SUCCESS
          jesse glick : e6d8500dd8e9b157ae8c54aec6fd42286e88301a
          Files :

          • pom.xml

          kohsuke : 9e321a4a0eefde421709647f0f15cb7bc8da61d1
          Files :

          • pom.xml

          dogfood added a comment - Integrated in jenkins_2.0 #5 [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix (Revision e6d8500dd8e9b157ae8c54aec6fd42286e88301a) [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix (Revision 9e321a4a0eefde421709647f0f15cb7bc8da61d1) Result = SUCCESS jesse glick : e6d8500dd8e9b157ae8c54aec6fd42286e88301a Files : pom.xml kohsuke : 9e321a4a0eefde421709647f0f15cb7bc8da61d1 Files : pom.xml

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

              Created:
              Updated:
              Resolved: