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

docker.build() should sanitize input for newlines

      Using the following basic Jenkinsfile I accidentally broke all my docker.build() invocations in a very difficult to grok bug.

      node('docker') {
          /* Make sure we're starting with a pristine workspace */
          deleteDir()
      
          String imageName = 'jenkinsciinfra/terraform'
          String commitHash
          def container = null
      
          stage('Checkout') {
              checkout scm
              commitHash = sh(returnStdout: true, script: 'git rev-parse --short origin/master')
          }
      
          stage('Build') {
              container = docker.build("${imageName}:${commitHash}")
          }
      }
      

      This would fail with:

      [Pipeline] // stage
      [Pipeline] stage
      [Pipeline] { (Build)
      [Pipeline] sh
      [terraform] Running shell script
      + docker build -t jenkinsciinfra/terraform:f84fd39
      "docker build" requires exactly 1 argument(s).
      See 'docker build --help'.
      
      Usage:  docker build [OPTIONS] PATH | URL | -
      
      Build an image from a Dockerfile
      [Pipeline] }
      [Pipeline] // stage
      

      The problem here is that the commitHash has a newline character hidden at the end of it; the fix is obviously to add .trim(), but the fact that docker.build() doesn't sanitize the imageName parameter means that the user isn't provided any useful feedback when the invalid input breaks the command.

      I would suggest either sanitizing the imageName input, or run some validation to ensure that imageName is valid.

          [JENKINS-41395] docker.build() should sanitize input for newlines

          Tim Downey added a comment -

          I just spent several hours getting burned by this. The easiest fix is to just change Docker.groovy below to trim both the image and the args, but it's possible that you'd want to go further with args, since any embedded newlines would be an issue. In my case, image was the issue with the newlines. I'd be happy to make the change and submit a PR if folks are happy with the solution.

              public Image build(String image, String args = '.') {
                  node {
                      // JENKINS-41395
                      image = image.trim()
                      args = args.trim()
          
                      def parsedArgs = args.split(/ (?=([^"']*["'][^"']*["'])*[^"']*$)/)
                      def dir = parsedArgs[-1] ?: '.'
          
                      // Detect custom Dockerfile:
                      def dockerfile = "${dir}/Dockerfile"
                      for (int i=0; i<parsedArgs.length; i++) {
                          if ((parsedArgs[i] == '-f' || parsedArgs[i] == '--file') && i < (parsedArgs.length - 1)) {
                              dockerfile = parsedArgs[i+1]
                              break
                          }
                      }
          
                      script.sh "docker build -t ${image} ${args}"
                      script.dockerFingerprintFrom dockerfile: dockerfile, image: image, toolName: script.env.DOCKER_TOOL_NAME
                      this.image(image)
                  }
              }
          

          Tim Downey added a comment - I just spent several hours getting burned by this. The easiest fix is to just change Docker.groovy below to trim both the image and the args, but it's possible that you'd want to go further with args, since any embedded newlines would be an issue. In my case, image was the issue with the newlines. I'd be happy to make the change and submit a PR if folks are happy with the solution. public Image build( String image, String args = '.' ) { node { // JENKINS-41395 image = image.trim() args = args.trim() def parsedArgs = args.split(/ (?=([^ " ']*[" ' ][^ " ']*[" ' ])*[^"']*$)/) def dir = parsedArgs[-1] ?: '.' // Detect custom Dockerfile: def dockerfile = "${dir}/Dockerfile" for ( int i=0; i<parsedArgs.length; i++) { if ((parsedArgs[i] == '-f' || parsedArgs[i] == '--file' ) && i < (parsedArgs.length - 1)) { dockerfile = parsedArgs[i+1] break } } script.sh "docker build -t ${image} ${args}" script.dockerFingerprintFrom dockerfile: dockerfile, image: image, toolName: script.env.DOCKER_TOOL_NAME this .image(image) } }

          Jesse Glick added a comment -

          Prefer validation with a clear error to quiet sanitization.

          Jesse Glick added a comment - Prefer validation with a clear error to quiet sanitization.

          pu lord added a comment -

          thanks

          pu lord added a comment - thanks

          Lifesaver.

          Just wasted an hour debugging this

           

          Thanks !!

          David Feldstein added a comment - Lifesaver. Just wasted an hour debugging this   Thanks !!

            Unassigned Unassigned
            rtyler R. Tyler Croy
            Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

              Created:
              Updated: