Skip to content

Commit 127116b

Browse files
committed
Remove broken frivolous function from install script
The "template" 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.
1 parent 0d9d4a9 commit 127116b

File tree

1 file changed

+16
-22
lines changed

1 file changed

+16
-22
lines changed

Diff for: other/installation-script/install.sh

+16-22
Original file line numberDiff line numberDiff line change
@@ -87,25 +87,6 @@ checkLatestVersion() {
8787
eval "$1='$CHECKLATESTVERSION_TAG'"
8888
}
8989

90-
get() {
91-
GET_URL="$2"
92-
echo "Getting $GET_URL"
93-
if [ "$DOWNLOAD_TOOL" = "curl" ]; then
94-
GET_HTTP_RESPONSE=$(curl -sL --write-out 'HTTPSTATUS:%{http_code}' "$GET_URL")
95-
GET_HTTP_STATUS_CODE=$(echo "$GET_HTTP_RESPONSE" | tr -d '\n' | sed -e 's/.*HTTPSTATUS://')
96-
GET_BODY=$(echo "$GET_HTTP_RESPONSE" | sed -e 's/HTTPSTATUS\:.*//g')
97-
elif [ "$DOWNLOAD_TOOL" = "wget" ]; then
98-
TMP_FILE=$(mktemp)
99-
GET_BODY=$(wget --server-response --content-on-error -q -O - "$GET_URL" 2>"$TMP_FILE" || true)
100-
GET_HTTP_STATUS_CODE=$(awk '/^ HTTP/{print $2}' "$TMP_FILE")
101-
fi
102-
if [ "$GET_HTTP_STATUS_CODE" != 200 ]; then
103-
echo "Request failed with HTTP status code $GET_HTTP_STATUS_CODE"
104-
fail "Body: $GET_BODY"
105-
fi
106-
eval "$1='$GET_BODY'"
107-
}
108-
10990
getFile() {
11091
GETFILE_URL="$1"
11192
GETFILE_FILE_PATH="$2"
@@ -147,11 +128,24 @@ downloadFile() {
147128
if [ "$httpStatusCode" -ne 200 ]; then
148129
echo "Did not find a release for your system: $OS $ARCH"
149130
echo "Trying to find a release using the GitHub API."
131+
150132
LATEST_RELEASE_URL="https://api.github.com/repos/${PROJECT_OWNER}/$PROJECT_NAME/releases/tags/$TAG"
151-
echo "LATEST_RELEASE_URL=$LATEST_RELEASE_URL"
152-
get LATEST_RELEASE_JSON "$LATEST_RELEASE_URL"
133+
if [ "$DOWNLOAD_TOOL" = "curl" ]; then
134+
HTTP_RESPONSE=$(curl -sL --write-out 'HTTPSTATUS:%{http_code}' "$LATEST_RELEASE_URL")
135+
HTTP_STATUS_CODE=$(echo "$HTTP_RESPONSE" | tr -d '\n' | sed -e 's/.*HTTPSTATUS://')
136+
BODY=$(echo "$HTTP_RESPONSE" | sed -e 's/HTTPSTATUS\:.*//g')
137+
elif [ "$DOWNLOAD_TOOL" = "wget" ]; then
138+
TMP_FILE=$(mktemp)
139+
BODY=$(wget --server-response --content-on-error -q -O - "$LATEST_RELEASE_URL" 2>"$TMP_FILE" || true)
140+
HTTP_STATUS_CODE=$(awk '/^ HTTP/{print $2}' "$TMP_FILE")
141+
fi
142+
if [ "$HTTP_STATUS_CODE" != 200 ]; then
143+
echo "Request failed with HTTP status code $HTTP_STATUS_CODE"
144+
fail "Body: $BODY"
145+
fi
146+
153147
# || true forces this command to not catch error if grep does not find anything
154-
DOWNLOAD_URL=$(echo "$LATEST_RELEASE_JSON" | grep 'browser_' | cut -d\" -f4 | grep "$APPLICATION_DIST") || true
148+
DOWNLOAD_URL=$(echo "$BODY" | grep 'browser_' | cut -d\" -f4 | grep "$APPLICATION_DIST") || true
155149
if [ -z "$DOWNLOAD_URL" ]; then
156150
echo "Sorry, we dont have a dist for your system: $OS $ARCH"
157151
fail "You can request one here: https://github.com/${PROJECT_OWNER}/$PROJECT_NAME/issues"

0 commit comments

Comments
 (0)