fix(wrapper): replace mktemp with POSIX-compliant temp dir creation in only-mvnw#418
fix(wrapper): replace mktemp with POSIX-compliant temp dir creation in only-mvnw#418armorbreak001 wants to merge 1 commit intoapache:masterfrom
Conversation
…n only-mvnw mktemp is not available in all POSIX shells (e.g., ksh, some bash versions on AIX). Use TMPDIR/PID/timestamp-based directory creation with mkdir -p instead, which works everywhere. Fixes apache#311
There was a problem hiding this comment.
Pull request overview
Updates the only-mvnw POSIX /bin/sh wrapper to avoid relying on mktemp -d for creating the temporary download directory, targeting better portability across non-GNU / non-Bash-default environments (e.g., AIX/ksh).
Changes:
- Replaces
mktemp -dtemp dir creation with aTMPDIR-based path andmkdir -p. - Adds a comment clarifying the intent to be POSIX-compatible.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _tmp_base="${TMPDIR:-/tmp}" | ||
| TMP_DOWNLOAD_DIR="$_tmp_base/maven_wrapper.$$_$(date +%s)" | ||
| if mkdir -p -- "$TMP_DOWNLOAD_DIR" && [ -d "$TMP_DOWNLOAD_DIR" ]; then |
There was a problem hiding this comment.
date +%s is not specified by POSIX and is missing on some platforms (incl. AIX), so this temp dir name generation can fail or produce non-unique names. Use a POSIX format (e.g., date '+%Y%m%d%H%M%S') and/or a small looped suffix counter to guarantee uniqueness without relying on %s.
| # prepare tmp dir (POSIX-compatible: avoid mktemp which is not available in all shells) | ||
| _tmp_base="${TMPDIR:-/tmp}" | ||
| TMP_DOWNLOAD_DIR="$_tmp_base/maven_wrapper.$$_$(date +%s)" | ||
| if mkdir -p -- "$TMP_DOWNLOAD_DIR" && [ -d "$TMP_DOWNLOAD_DIR" ]; then | ||
| clean() { rm -rf -- "$TMP_DOWNLOAD_DIR"; } |
There was a problem hiding this comment.
The temp directory path is predictable and mkdir -p will succeed if it already exists, which can lead to reusing a pre-existing directory (DoS) and enables TOCTOU/symlink attacks on files created inside it (e.g., Downloader.java, the downloaded archive). Prefer creating a new directory with mkdir (no -p) in a loop until it succeeds, set a restrictive umask/mode, and consider using mktemp -d when available with a POSIX fallback when it isn’t.
Summary
Fixes #311
only-mvnwdeclares#!/bin/shshebang but usesmktemp -d, which is not POSIX-compliant and not available in all shells (e.g., ksh, some bash versions on AIX).Changes
Replace
mktemp -dwith a portable alternative usingTMPDIR, PID ($$), and epoch timestamp to construct a unique temp directory path, then create it withmkdir -p.This approach:
trap clean EXITTMPDIRlike the original code didTesting
mvnwwrapper script