From 346094bf4400b074648b28abb9994e7647151526 Mon Sep 17 00:00:00 2001 From: per1234 Date: Mon, 2 May 2022 03:02:11 -0700 Subject: [PATCH 1/3] Remove broken frivolous function from install script The installation script contained a `get()` function used to make an HTTP request. The response body is required by the caller, but shell functions do not support returning content as you might do when using a more capable programming language. As a workaround, the script author set up a system where the caller passed an arbitrary variable name to the function, which the function then assigns with the response body string. This was done using the `eval` builtin. The `eval` command was quoted in a naive manner, not considering that the body content could contain any arbitrary characters. This made it possible that contents of the response body could be unintentionally executed on the user's machine. In the context of the installation script, this happened under the following conditions: - The release download was not available from Arduino's downloads server - The response body from the Releases GitHub API request contained single quotes The most minimal fix would likely have been to change the `eval` command so that the variable containing the response body text was not expanded in the command: eval "$1=\$GET_BODY" However, this problem has provided clear evidence that best practices are to avoid `eval` altogether unless absolutely necessary. Since it is only called once, the entire function is not necessary (and in fact it is questionable whether there is any real value in the entire GitHub releases fallback system). So I decided the best fix was to do away with the function altogether, replacing its single call with the contents of the former function. This removed unnecessary complexity from the script without any decrease in efficiency or increase in maintenance burden. --- etc/install.sh | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/etc/install.sh b/etc/install.sh index cdbe9be4..ae7b5fa4 100755 --- a/etc/install.sh +++ b/etc/install.sh @@ -87,25 +87,6 @@ checkLatestVersion() { eval "$1='$CHECKLATESTVERSION_TAG'" } -get() { - GET_URL="$2" - echo "Getting $GET_URL" - if [ "$DOWNLOAD_TOOL" = "curl" ]; then - GET_HTTP_RESPONSE=$(curl -sL --write-out 'HTTPSTATUS:%{http_code}' "$GET_URL") - GET_HTTP_STATUS_CODE=$(echo "$GET_HTTP_RESPONSE" | tr -d '\n' | sed -e 's/.*HTTPSTATUS://') - GET_BODY=$(echo "$GET_HTTP_RESPONSE" | sed -e 's/HTTPSTATUS\:.*//g') - elif [ "$DOWNLOAD_TOOL" = "wget" ]; then - TMP_FILE=$(mktemp) - GET_BODY=$(wget --server-response --content-on-error -q -O - "$GET_URL" 2>"$TMP_FILE" || true) - GET_HTTP_STATUS_CODE=$(awk '/^ HTTP/{print $2}' "$TMP_FILE") - fi - if [ "$GET_HTTP_STATUS_CODE" != 200 ]; then - echo "Request failed with HTTP status code $GET_HTTP_STATUS_CODE" - fail "Body: $GET_BODY" - fi - eval "$1='$GET_BODY'" -} - getFile() { GETFILE_URL="$1" GETFILE_FILE_PATH="$2" @@ -147,11 +128,24 @@ downloadFile() { if [ "$httpStatusCode" -ne 200 ]; then echo "Did not find a release for your system: $OS $ARCH" echo "Trying to find a release using the GitHub API." + LATEST_RELEASE_URL="https://api.github.com/repos/${PROJECT_OWNER}/$PROJECT_NAME/releases/tags/$TAG" - echo "LATEST_RELEASE_URL=$LATEST_RELEASE_URL" - get LATEST_RELEASE_JSON "$LATEST_RELEASE_URL" + if [ "$DOWNLOAD_TOOL" = "curl" ]; then + HTTP_RESPONSE=$(curl -sL --write-out 'HTTPSTATUS:%{http_code}' "$LATEST_RELEASE_URL") + HTTP_STATUS_CODE=$(echo "$HTTP_RESPONSE" | tr -d '\n' | sed -e 's/.*HTTPSTATUS://') + BODY=$(echo "$HTTP_RESPONSE" | sed -e 's/HTTPSTATUS\:.*//g') + elif [ "$DOWNLOAD_TOOL" = "wget" ]; then + TMP_FILE=$(mktemp) + BODY=$(wget --server-response --content-on-error -q -O - "$LATEST_RELEASE_URL" 2>"$TMP_FILE" || true) + HTTP_STATUS_CODE=$(awk '/^ HTTP/{print $2}' "$TMP_FILE") + fi + if [ "$HTTP_STATUS_CODE" != 200 ]; then + echo "Request failed with HTTP status code $HTTP_STATUS_CODE" + fail "Body: $BODY" + fi + # || true forces this command to not catch error if grep does not find anything - DOWNLOAD_URL=$(echo "$LATEST_RELEASE_JSON" | grep 'browser_' | cut -d\" -f4 | grep "$APPLICATION_DIST") || true + DOWNLOAD_URL=$(echo "$BODY" | grep 'browser_' | cut -d\" -f4 | grep "$APPLICATION_DIST") || true if [ -z "$DOWNLOAD_URL" ]; then echo "Sorry, we dont have a dist for your system: $OS $ARCH" fail "You can request one here: https://github.com/${PROJECT_OWNER}/$PROJECT_NAME/issues" From aaad5fbba5cad8b70d1c97fa69c2b67cee74e42d Mon Sep 17 00:00:00 2001 From: per1234 Date: Mon, 2 May 2022 03:03:54 -0700 Subject: [PATCH 2/3] Fallback to x86-64 release when macOS ARM 64-bit build not available Apple's Rosetta 2 software allows applications built for x86-64 hosts to run on machines using their ARM 64-bit M1 processor. Due to current infrastructure challenges (e.g., lack of GitHub-hosted GitHub Actions runners), Arduino Lint is not yet available as a native M1 build, yet the existing macOS x86-64 builds work perfectly well on these machines thanks to Rosetta 2. The installation script previously failed to install Arduino Lint when ran on the M1 host: Did not find a release for your system: macOS arm64 --- etc/install.sh | 84 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 56 insertions(+), 28 deletions(-) diff --git a/etc/install.sh b/etc/install.sh index ae7b5fa4..83f7cc3e 100755 --- a/etc/install.sh +++ b/etc/install.sh @@ -40,6 +40,7 @@ initArch() { armv6*) ARCH="ARMv6" ;; armv7*) ARCH="ARMv7" ;; aarch64) ARCH="ARM64" ;; + arm64) ARCH="ARM64" ;; x86) ARCH="32bit" ;; x86_64) ARCH="64bit" ;; i686) ARCH="32bit" ;; @@ -48,6 +49,15 @@ initArch() { echo "ARCH=$ARCH" } +initFallbackArch() { + case "${OS}_${ARCH}" in + macOS_ARM64) + # Rosetta 2 allows applications built for x86-64 hosts to run on the ARM 64-bit M1 processor + FALLBACK_ARCH='64bit' + ;; + esac +} + initOS() { OS=$(uname -s) case "$OS" in @@ -106,52 +116,69 @@ downloadFile() { TAG=$1 fi # arduino-lint_0.4.0-rc1_Linux_64bit.[tar.gz, zip] + APPLICATION_DIST_PREFIX="${PROJECT_NAME}_${TAG}_" if [ "$OS" = "Windows" ]; then - APPLICATION_DIST="${PROJECT_NAME}_${TAG}_${OS}_${ARCH}.zip" + APPLICATION_DIST_EXTENSION=".zip" else - APPLICATION_DIST="${PROJECT_NAME}_${TAG}_${OS}_${ARCH}.tar.gz" + APPLICATION_DIST_EXTENSION=".tar.gz" fi + APPLICATION_DIST="${APPLICATION_DIST_PREFIX}${OS}_${ARCH}${APPLICATION_DIST_EXTENSION}" # Support specifying nightly build versions (e.g., "nightly-latest") via the script argument. case "$TAG" in nightly*) - DOWNLOAD_URL="https://downloads.arduino.cc/${PROJECT_NAME}/nightly/${APPLICATION_DIST}" + DOWNLOAD_URL_PREFIX="https://downloads.arduino.cc/${PROJECT_NAME}/nightly/" ;; *) - DOWNLOAD_URL="https://downloads.arduino.cc/${PROJECT_NAME}/${APPLICATION_DIST}" + DOWNLOAD_URL_PREFIX="https://downloads.arduino.cc/${PROJECT_NAME}/" ;; esac + DOWNLOAD_URL="${DOWNLOAD_URL_PREFIX}${APPLICATION_DIST}" INSTALLATION_TMP_FILE="/tmp/$APPLICATION_DIST" echo "Downloading $DOWNLOAD_URL" httpStatusCode=$(getFile "$DOWNLOAD_URL" "$INSTALLATION_TMP_FILE") if [ "$httpStatusCode" -ne 200 ]; then - echo "Did not find a release for your system: $OS $ARCH" - echo "Trying to find a release using the GitHub API." - - LATEST_RELEASE_URL="https://api.github.com/repos/${PROJECT_OWNER}/$PROJECT_NAME/releases/tags/$TAG" - if [ "$DOWNLOAD_TOOL" = "curl" ]; then - HTTP_RESPONSE=$(curl -sL --write-out 'HTTPSTATUS:%{http_code}' "$LATEST_RELEASE_URL") - HTTP_STATUS_CODE=$(echo "$HTTP_RESPONSE" | tr -d '\n' | sed -e 's/.*HTTPSTATUS://') - BODY=$(echo "$HTTP_RESPONSE" | sed -e 's/HTTPSTATUS\:.*//g') - elif [ "$DOWNLOAD_TOOL" = "wget" ]; then - TMP_FILE=$(mktemp) - BODY=$(wget --server-response --content-on-error -q -O - "$LATEST_RELEASE_URL" 2>"$TMP_FILE" || true) - HTTP_STATUS_CODE=$(awk '/^ HTTP/{print $2}' "$TMP_FILE") - fi - if [ "$HTTP_STATUS_CODE" != 200 ]; then - echo "Request failed with HTTP status code $HTTP_STATUS_CODE" - fail "Body: $BODY" + if [ -n "$FALLBACK_ARCH" ]; then + echo "$OS $ARCH release not currently available. Checking for alternative $OS $FALLBACK_ARCH release for your system." + FALLBACK_APPLICATION_DIST="${APPLICATION_DIST_PREFIX}${OS}_${FALLBACK_ARCH}${APPLICATION_DIST_EXTENSION}" + DOWNLOAD_URL="${DOWNLOAD_URL_PREFIX}${FALLBACK_APPLICATION_DIST}" + echo "Downloading $DOWNLOAD_URL" + httpStatusCode=$(getFile "$DOWNLOAD_URL" "$INSTALLATION_TMP_FILE") fi - # || true forces this command to not catch error if grep does not find anything - DOWNLOAD_URL=$(echo "$BODY" | grep 'browser_' | cut -d\" -f4 | grep "$APPLICATION_DIST") || true - if [ -z "$DOWNLOAD_URL" ]; then - echo "Sorry, we dont have a dist for your system: $OS $ARCH" - fail "You can request one here: https://github.com/${PROJECT_OWNER}/$PROJECT_NAME/issues" - else - echo "Downloading $DOWNLOAD_URL" - getFile "$DOWNLOAD_URL" "$INSTALLATION_TMP_FILE" + if [ "$httpStatusCode" -ne 200 ]; then + echo "Did not find a release for your system: $OS $ARCH" + echo "Trying to find a release using the GitHub API." + + LATEST_RELEASE_URL="https://api.github.com/repos/${PROJECT_OWNER}/$PROJECT_NAME/releases/tags/$TAG" + if [ "$DOWNLOAD_TOOL" = "curl" ]; then + HTTP_RESPONSE=$(curl -sL --write-out 'HTTPSTATUS:%{http_code}' "$LATEST_RELEASE_URL") + HTTP_STATUS_CODE=$(echo "$HTTP_RESPONSE" | tr -d '\n' | sed -e 's/.*HTTPSTATUS://') + BODY=$(echo "$HTTP_RESPONSE" | sed -e 's/HTTPSTATUS\:.*//g') + elif [ "$DOWNLOAD_TOOL" = "wget" ]; then + TMP_FILE=$(mktemp) + BODY=$(wget --server-response --content-on-error -q -O - "$LATEST_RELEASE_URL" 2>"$TMP_FILE" || true) + HTTP_STATUS_CODE=$(awk '/^ HTTP/{print $2}' "$TMP_FILE") + fi + if [ "$HTTP_STATUS_CODE" != 200 ]; then + echo "Request failed with HTTP status code $HTTP_STATUS_CODE" + fail "Body: $BODY" + fi + + # || true forces this command to not catch error if grep does not find anything + DOWNLOAD_URL=$(echo "$BODY" | grep 'browser_' | cut -d\" -f4 | grep "$APPLICATION_DIST") || true + if [ -z "$DOWNLOAD_URL" ]; then + DOWNLOAD_URL=$(echo "$BODY" | grep 'browser_' | cut -d\" -f4 | grep "$FALLBACK_APPLICATION_DIST") || true + fi + + if [ -z "$DOWNLOAD_URL" ]; then + echo "Sorry, we dont have a dist for your system: $OS $ARCH" + fail "You can request one here: https://github.com/${PROJECT_OWNER}/$PROJECT_NAME/issues" + else + echo "Downloading $DOWNLOAD_URL" + getFile "$DOWNLOAD_URL" "$INSTALLATION_TMP_FILE" + fi fi fi } @@ -208,6 +235,7 @@ initDestination set -e initArch initOS +initFallbackArch initDownloadTool downloadFile "$1" installFile From 0e9c6121c50c982d1d2f961b700e7e0576414bc4 Mon Sep 17 00:00:00 2001 From: per1234 Date: Mon, 2 May 2022 03:05:14 -0700 Subject: [PATCH 3/3] Remove unnecessary use of `eval` from install script The installation script contains a `checkLatestVersion()` function which is used to determine the latest version of the project when the user does not specify a version to install. The version number is required by the caller, but shell functions do not support returning such content. As a workaround, the script author set up a system where the caller passed an arbitrary variable name to the function. The function then sets a global variable of that name with the release name. So it resembles a "pass by reference" approach, but isn't. This was done using the `eval` builtin. This tool must be used with caution and best practices is to avoid it unless absolutely necessary. The system used by the script has some value for a reusable function. However, in this case the function is only intended for internal use by the script, and is called only once. So the unintuitive nature of this system and potential for bugs don't bring any benefits when compared with the much more straightforward approach of simply using a fixed name for the global variable used to store the release name. --- etc/install.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/etc/install.sh b/etc/install.sh index 83f7cc3e..a450a9fd 100755 --- a/etc/install.sh +++ b/etc/install.sh @@ -80,6 +80,7 @@ initDownloadTool() { echo "Using $DOWNLOAD_TOOL as download tool" } +# checkLatestVersion() sets the CHECKLATESTVERSION_TAG variable to the latest version checkLatestVersion() { # Use the GitHub releases webpage to find the latest version for this project # so we don't get rate-limited. @@ -94,7 +95,6 @@ checkLatestVersion() { echo "Cannot determine latest tag." exit 1 fi - eval "$1='$CHECKLATESTVERSION_TAG'" } getFile() { @@ -111,7 +111,8 @@ getFile() { downloadFile() { if [ -z "$1" ]; then - checkLatestVersion TAG + checkLatestVersion + TAG="$CHECKLATESTVERSION_TAG" else TAG=$1 fi