Merge pull request #863 from srvrco/fix-json_get-parse-error

Fix test failure when token contains the phrase "url"

getssl

@@ -1424,10 +1424,8 @@ for d in "${alldomains[@]}"; do
       else # APIv2
         debug "authlink response = ${response//[$'\t\r\n']}"
         # get the token and uri from the dns-01 component
-        token=$(json_get "$response" "challenges" "type" "dns-01" "token")
-        uri=$(json_get "$response" "challenges" "type" "dns-01" "url")
-        # when using pebble this sometimes appears to have a newline which causes problems in send_signed_request
-        uri=$(echo "$uri" | tr -d '\r')
+        token=$(json_get "$response" "challenges" "type" "dns-01" '"token"')
+        uri=$(json_get "$response" "challenges" "type" "dns-01" '"url"')
         debug uri "$uri"
       fi
 
@@ -1488,9 +1486,9 @@ for d in "${alldomains[@]}"; do
       else # APIv2
         debug "authlink response = ${response//[$'\t\r\n']}"
         # get the token from the http-01 component
-        token=$(json_get "$response" "challenges" "type" "http-01" "token")
+        token=$(json_get "$response" "challenges" "type" "http-01" '"token"')
         # get the uri from the http component
-        uri=$(json_get "$response" "challenges" "type" "http-01" "url" | head -n1)
+        uri=$(json_get "$response" "challenges" "type" "http-01" '"url"' | head -n1)
         debug uri "$uri"
       fi
 

test/test_helper.bash

@@ -61,9 +61,27 @@ check_nginx() {
   fi
 }
 
+whitelist_array=(
+  "DNS problem"
+  "acme:error:badNonce"
+  "acme:error:dns"
+)
+
 check_output_for_errors() {
+  # check if the output contains a whitelisted phrase, if it does, don't check for the phrase "Error"
+  contains_whitelisted_phrase=0
+  for phrase in "${whitelist_array[@]}"; do
+    #echo "# DEBUG: checking output for whitelisted phrase: $phrase"
+    status=1
+    assert_output --regexp "$phrase" 2>/dev/null || status=0
+    contains_whitelisted_phrase=$((status || contains_whitelisted_phrase))
+  done
+
+  if [[ $contains_whitelisted_phrase -eq 0 ]]; then
+    refute_output --regexp '[^_][Ee][Rr][Rr][Oo][Rr]'
+  fi
+
   refute_output --regexp '[Ff][Aa][Ii][Ll][Ee][Dd]'
-  refute_output --regexp '[^_][Ee][Rr][Rr][Oo][Rr][^:badNonce]'
   refute_output --regexp '[^_][Ww][Aa][Rr][Nn][Ii][Nn][Gg]'
   refute_line --partial 'command not found'
 }
@@ -107,9 +125,9 @@ setup_environment() {
 # shellcheck disable=SC2153 # Ignore GETSSL_OS looks like typo of GETSSL_IP
 if [[ -f /usr/bin/supervisord && -f /etc/supervisord.conf ]]; then
   if [[ ! $(pgrep supervisord) ]]; then
-   /usr/bin/supervisord -c /etc/supervisord.conf 3>&- 4>&-
-   # Give supervisord time to start
-   sleep 1
+    /usr/bin/supervisord -c /etc/supervisord.conf 3>&- 4>&-
+    # Give supervisord time to start
+    sleep 1
   fi
 elif [[ "$GETSSL_OS" == "centos"[78] || "$GETSSL_OS" == "rockylinux"* ]]; then
   if [ -z "$(pgrep nginx)" ]; then

test/u10-test-json_get.bats

@@ -0,0 +1,96 @@
+#! /usr/bin/env bats
+
+load '/bats-support/load.bash'
+load '/bats-assert/load.bash'
+load '/getssl/test/test_helper.bash'
+
+
+# This is run for every test
+setup() {
+    [ ! -f $BATS_RUN_TMPDIR/failed.skip ] || skip "skipping tests after first failure"
+
+    . /getssl/getssl --source
+    export API=2
+    _USE_DEBUG=1
+}
+
+
+teardown() {
+    [ -n "$BATS_TEST_COMPLETED" ] || touch $BATS_RUN_TMPDIR/failed.skip
+}
+
+response='
+{
+    "challenges": [
+        {
+            "status": "pending",
+            "token": "kD1H4FVIEFvkWghLlKFoSPpR5u0FTGkRs4A_FnTfv3A",
+            "type": "http-01",
+            "url": "https://pebble:14000/chalZ/firw72KAYbsChpxMAzrTSLpCKepAdqcJn7NERZtAknY"
+        },
+        {
+            "status": "pending",
+            "token": "3FMfZoNNrjZzh_nnxanW5oEKvC6urlGS5wQWI5Bg9J4",
+            "type": "dns-01",
+            "url": "https://pebble:14000/chalZ/vkHAS1A9tQQ5A8QoAIRQJrSC_WJNm303iwC1r22dnCc"
+        },
+        {
+            "status": "pending",
+            "token": "UGkg34cDGoM9Su22iCH9yn383uLfTpr5Ys4Tms9QYAo",
+            "type": "dns-account-01",
+            "url": "https://pebble:14000/chalZ/ryNLsf-iOe22YYeYv6YIwBp7E2z492bdesvNQFzl9gI"
+        },
+        {
+            "status": "pending",
+            "token": "Sla6q_0Nl3JB3JMsWCXn_X3-KyH45mjKaStRDZU8I0g",
+            "type": "tls-alpn-01",
+            "url": "https://pebble:14000/chalZ/pzLqpT2qVf4DxK25GX0mONLE9Ii35FAXL9ioxONoSFQ"
+        }
+    ],
+    "expires": "2024-10-18T17:24:42Z",
+    "identifier": {
+        "type": "dns",
+        "value": "c.debian.getssl.test"
+    },
+    "status": "pending"
+}'
+
+
+@test "Test that json_get fails if token contains the phrase 'url'" {
+    # the token for te dns-01 entry contains the text "url" which breaks the json_get url parser!
+
+    type="dns-01"
+    uri=$(json_get "$response" "challenges" "type" $type "url")
+    token=$(json_get "$response" "challenges" "type" $type "token")
+    # when using pebble this sometimes appears to have a newline which causes problems in send_signed_request
+    uri=$(echo "$uri" | tr -d '\r')
+    echo uri "$uri" >&3
+    echo token "$token" >&3
+
+    # check the uri begins with https
+    begins_with_https=0
+    if [[ "$uri" == https* ]]; then
+        begins_with_https=1
+    fi
+
+    assert_not_equal $begins_with_https 1
+}
+
+
+@test "Test that json_get works if we quote 'url'" {
+    # the token for te dns-01 entry contains the text "url" which breaks the json_get url parser!
+
+    type="dns-01"
+    uri=$(json_get "$response" "challenges" "type" $type '"url"')
+    token=$(json_get "$response" "challenges" "type" $type '"token"')
+    echo uri "$uri" >&3
+    echo token "$token" >&3
+
+    # check the uri begins with https
+    begins_with_https=0
+    if [[ "$uri" == https* ]]; then
+        begins_with_https=1
+    fi
+
+    assert_equal $begins_with_https 1
+}

test/u11-test-ignore-dns-error-in-output.bats

@@ -0,0 +1,88 @@
+#! /usr/bin/env bats
+
+load '/bats-support/load.bash'
+load '/bats-assert/load.bash'
+load '/getssl/test/test_helper.bash'
+
+
+# This is run for every test
+setup() {
+    [ ! -f $BATS_RUN_TMPDIR/failed.skip ] || skip "skipping tests after first failure"
+
+    . /getssl/getssl --source
+    export API=2
+    _USE_DEBUG=1
+}
+
+
+teardown() {
+    [ -n "$BATS_TEST_COMPLETED" ] || touch $BATS_RUN_TMPDIR/failed.skip
+}
+
+# First sample text where we don't want check_output_for_errors to find an error
+output1=(
+'send_signed_request:533 code 200'
+'send_signed_request:533 response status = invalid'
+'check_challenge_completion:1472 *.ubuntu-acmedns-getssl.freeddns.org:Verify error:    "detail": "DNS problem: server failure at resolver looking up CAA for freeddns.org",'
+'del_dns_rr:1474 removing DNS RR via command: /getssl/dns_scripts/dns_del_acmedns ubuntu-acmedns-getssl.freeddns.org hEAib3ePU0s8-G3HPmPSa50ZjfdKt0A0qskHyTfBJr8'
+'send_signed_request:1215 url https://acme-staging-v02.api.letsencrypt.org/acme/new-order'
+'send_signed_request:1215 using KID=https://acme-staging-v02.api.letsencrypt.org/acme/acct/168360453'
+'send_signed_request:1215 payload = {"identifiers": [{"type":"dns","value":"*.ubuntu-acmedns-getssl.freeddns.org"}]}'
+)
+
+# Second sample text where we don't want check_output_for_errors to find an error
+output2=(
+'send_signed_request:3553 response {  "identifier": {    "type": "dns",    "value": "ubuntu-acmedns-getssl.freeddns.org"  },  "status": "invalid",  "expires": "2024-10-30T15:24:16Z",  "challenges": [    {      "type": "dns-01",      "url": "https://acme-staging-v02.api.letsencrypt.org/acme/chall-v3/14558038743/zzz8VA",      "status": "invalid",      "validated": "2024-10-23T15:24:18Z",      "error": {        "type": "urn:ietf:params:acme:error:dns",        "detail": "DNS problem: server failure at resolver looking up CAA for freeddns.org",        "status": 400      },      "token": "PyBVfKevM4noXq3fdsFs_0G1BY_o7Nl7eGa6mQw7oJM",      "validationRecord": [        {          "hostname": "ubuntu-acmedns-getssl.freeddns.org"        }      ]    }  ],  "wildcard": true}'
+'send_signed_request:3553 code 200'
+'send_signed_request:3553 response status = invalid'
+'main:0 deactivating domain'
+'main:0 deactivating  https://acme-staging-v02.api.letsencrypt.org/acme/authz-v3/14558038743'
+'send_signed_request:3557 url https://acme-staging-v02.api.letsencrypt.org/acme/authz-v3/14558038743'
+'send_signed_request:3557 using KID=https://acme-staging-v02.api.letsencrypt.org/acme/acct/168360453'
+'send_signed_request:3557 payload = {"resource": "authz", "status": "deactivated"}'
+)
+
+# Text that should cause check_output_for_errors to find an error
+output3=(
+'send_signed_request:3553 code 200'
+'send_signed_request:3553 response status = error'
+'main:0 deactivating domain'
+'main:0 deactivating  https://acme-staging-v02.api.letsencrypt.org/acme/authz-v3/14558038743'
+'send_signed_request:3557 url https://acme-staging-v02.api.letsencrypt.org/acme/authz-v3/14558038743'
+'send_signed_request:3557 using KID=https://acme-staging-v02.api.letsencrypt.org/acme/acct/168360453'
+'send_signed_request:3557 payload = {"resource": "authz", "status": "deactivated"}'
+)
+
+
+output_test_text() {
+    input_array=("$@")
+    printf '%s\n' "${input_array[@]}"
+}
+
+
+@test "Test that 'Verify error...DNS problem'  in first sample output is ignored" {
+    # print the known output that used to break the check
+    run output_test_text "${output1[@]}"
+
+    # run the check
+    check_output_for_errors
+}
+
+
+@test "Test that 'acme:dns:error' in second sample output is ignored" {
+    # print the known output that used to break the check
+    run output_test_text "${output2[@]}"
+
+    # run the check
+    check_output_for_errors
+}
+
+
+@test "Test that generic error in third sample output is NOT ignored" {
+    # print sample output that should cause 'check_output_for_errors' to fail a test
+    run output_test_text "${output3[@]}"
+
+    # run the function and check the output confirms that it would fail the test
+    run check_output_for_errors
+    assert_output --partial "-- regular expression should not match output --"
+}