Skip to content

fix(php-update): remove redundant duplicate tracking and narrow over-broad sed regexes - #229

Merged
PDowney merged 3 commits into
masterfrom
copilot/remove-redundant-seen-old-php-vers
May 2, 2026
Merged

fix(php-update): remove redundant duplicate tracking and narrow over-broad sed regexes#229
PDowney merged 3 commits into
masterfrom
copilot/remove-redundant-seen-old-php-vers

Conversation

Copilot AI commented May 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Five overly-broad sed substitutions in php-update.sh could silently corrupt comments, string literals, and unrelated version references during PHP migration. The SEEN_OLD_PHP_VERS deduplication was also redundant against SUPPORTED_PHP_VERSIONS.

Software Version Updates

No version changes.

Changed Versions

N/A

Version Diff

- declare -A SEEN_OLD_PHP_VERS=()
- if [[ -z "${SEEN_OLD_PHP_VERS["${ver}"]+x}" ]]; then
-     OLD_PHP_VERS+=("${ver}")
-     SEEN_OLD_PHP_VERS["${ver}"]=1
- fi
+ OLD_PHP_VERS+=("${ver}")

# php-fpm.conf: targeted socket + fastcgi_pass patterns
- sed -E -i "s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\1|g" "/etc/nginx/globals/php-fpm.conf"
+ sed -E -i \
+     -e "s|(unix:/run/php/)php${OLD_VER}-fpm(\.sock)|\1php${NEW_PHP_VER}-fpm\2|g" \
+     -e "s|(fastcgi_pass[[:space:]]+[^;]*php)${OLD_VER}(-fpm)|\1${NEW_PHP_VER}\2|g" \
+     "/etc/nginx/globals/php-fpm.conf"

# nginx sites: address-filtered to fastcgi_pass lines only
- sed -E -i "s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\1|g" "$config_file"
+ sed -E -i "/^[[:space:]]*fastcgi_pass[[:space:]]+/ s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\1|g" "$config_file"

# phpsysinfo.ini: anchored to key = value lines
- sed -E -i "s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\1|g" ".../phpsysinfo.ini"
+ sed -E -i "s|^([[:space:]]*[[:alnum:]_.-]+[[:space:]]*=[[:space:]]*.*)php${OLD_VER}(-fpm)?|\1php${NEW_PHP_VER}\2|g" ".../phpsysinfo.ini"

# api.php: restricted to fpm/socket/service-referencing lines
- sed -E -i "s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\1|g" ".../api.php"
+ sed -E -i "/(php-fpm|fastcgi_pass|sock(et)?|service)/ s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\1|g" ".../api.php"

Verification Checklist

  • Version numbers are correctly formatted
  • Version numbers are stable releases (not beta/RC)
  • No syntax errors in variables file

Notes

Each replacement is now scoped to its relevant directive or value context, eliminating the risk of touching commented-out lines, documentation blocks, or unrelated PHP version references during migration.

Original prompt 
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The SEEN_OLD_PHP_VERS associative array is redundant since SUPPORTED_PHP_VERSIONS should already contain unique values, and the logic only appends versions not equal to NEW_PHP_VER. The duplicate tracking adds unnecessary complexity without providing value.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -36,13 +36,9 @@\n \n # Auto-detect currently installed PHP-FPM version\n OLD_PHP_VERS=()\n-declare -A SEEN_OLD_PHP_VERS=()\n for ver in \"${SUPPORTED_PHP_VERSIONS[@]}\"; do\n     if [[ \"${ver}\" != \"${NEW_PHP_VER}\" ]] && dpkg -l | grep -q \"php${ver}-fpm\"; then\n-        if [[ -z \"${SEEN_OLD_PHP_VERS[\"${ver}\"]+x}\" ]]; then\n-            OLD_PHP_VERS+=(\"${ver}\")\n-            SEEN_OLD_PHP_VERS[\"${ver}\"]=1\n-        fi\n+        OLD_PHP_VERS+=(\"${ver}\")\n     fi\n done\n \n"}]},{"message":"The regex pattern replaces all occurrences of OLD_VER in the file, which could inadvertently replace OLD_VER in comments, documentation, or unrelated context. The pattern should be more specific to target only socket paths or service references to avoid unintended replacements.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -136,7 +136,10 @@\n # Update php-fpm.conf\n if [[ -f \"/etc/nginx/globals/php-fpm.conf\" ]]; then\n     for OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n-        sed -E -i \"s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\\1|g\" \"/etc/nginx/globals/php-fpm.conf\"\n+        sed -E -i \\\n+            -e \"s|(unix:/run/php/)php${OLD_VER}-fpm(\\.sock)|\\1php${NEW_PHP_VER}-fpm\\2|g\" \\\n+            -e \"s|(fastcgi_pass[[:space:]]+[^;]*php)${OLD_VER}(-fpm)|\\1${NEW_PHP_VER}\\2|g\" \\\n+            \"/etc/nginx/globals/php-fpm.conf\"\n     done\n fi\n \n@@ -144,7 +147,10 @@\n for config_file in /etc/nginx/sites-available/*; do\n     if [[ -f \"$config_file\" ]]; then\n         for OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n-            sed -E -i \"s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\\1|g\" \"$config_file\"\n+            sed -E -i \\\n+                -e \"s|(unix:/run/php/)php${OLD_VER}-fpm(\\.sock)|\\1php${NEW_PHP_VER}-fpm\\2|g\" \\\n+                -e \"s|(fastcgi_pass[[:space:]]+[^;]*php)${OLD_VER}(-fpm)|\\1${NEW_PHP_VER}\\2|g\" \\\n+                \"$config_file\"\n         done\n     fi\n done\n"}]},{"message":"Similar to the php-fpm.conf replacement, this broad regex could replace OLD_VER in comments or other non-configuration contexts within nginx site files. Consider using a more targeted pattern that specifically matches socket paths or fastcgi_pass directives.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -144,7 +144,7 @@\n for config_file in /etc/nginx/sites-available/*; do\n     if [[ -f \"$config_file\" ]]; then\n         for OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n-            sed -E -i \"s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\\1|g\" \"$config_file\"\n+            sed -E -i \"/^[[:space:]]*fastcgi_pass[[:space:]]+/ s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\\1|g\" \"$config_file\"\n         done\n     fi\n done\n"}]},{"message":"The same overly broad regex pattern is used here, which may replace OLD_VER in unintended locations within the phpsysinfo.ini file. Use a more specific pattern targeting actual configuration values rather than all occurrences.","fixFiles":[{"filePath":"scripts/update/php-update.sh","diff":"diff --git a/scripts/update/php-update.sh b/scripts/update/php-update.sh\n--- a/scripts/update/php-update.sh\n+++ b/scripts/update/php-update.sh\n@@ -153,7 +153,7 @@\n if [[ -f \"/var/www/admin/tools/phpsysinfo/phpsysinfo.ini\" ]]; then\n     echo \"Updating phpSysInfo configuration...\"\n     for OLD_VER in \"${MIGRATION_SOURCE_PHP_VERS[@]}\"; do\n-        sed -E -i \"s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\\1|g\" \"/var/www/admin/tools/phpsysinfo/phpsysinfo.ini\"\n+        sed -E -i \"s|^([[:space:]]*[[:alnum:]_.-]+[[:space:]]*=[[:space:]]*.*)php${OLD_VER}(-fpm)?|\\1php${NEW_PHP_VER}\\2|g\" \"/var/www/admin/tools/phpsysinfo/phpsysinfo.ini\"\n     done\n fi\n \n"}]},{"message":"Applying a global regex replacement to a PHP source file is risky as it could replace version strings in comments, string literals, or variable names unrelated to the PHP-FPM configuration. Target only the specific configuration lines that reference the PHP-FPM socket or service.","fixFiles":[{"filePath"...

Copilot AI changed the title [WIP] Remove redundant SEEN_OLD_PHP_VERS in PHP update script fix(php-update): remove redundant duplicate tracking and narrow over-broad sed regexes May 2, 2026
Copilot finished work on behalf of PDowney May 2, 2026 05:27
Copilot AI requested a review from PDowney May 2, 2026 05:27
@PDowney
Update CHANGELOG with recent PHP updates and fixes
e201340 
Removed redundant PHP version tracking and improved regex replacements for configuration files to prevent unintended changes.
@github-actions

github-actions Bot commented May 2, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@sonarqubecloud

sonarqubecloud Bot commented May 2, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@PDowney
PDowney marked this pull request as ready for review May 2, 2026 06:12
Copilot AI review requested due to automatic review settings May 2, 2026 06:12
@PDowney
PDowney merged commit bd17f1c into master May 2, 2026
12 checks passed
@github-actions
github-actions Bot deleted the copilot/remove-redundant-seen-old-php-vers branch May 2, 2026 06:13
Copilot AI reviewed May 2, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens the PHP upgrade script so version rewrites during PHP migrations are more narrowly scoped, aiming to reduce accidental edits to unrelated text in deployed config files while simplifying old-version detection.

Changes:

  • Removes redundant duplicate tracking when collecting installed legacy PHP-FPM versions.
  • Replaces broad global sed substitutions with more targeted patterns for Nginx, phpSysInfo, and admin control panel files.
  • Limits some replacements to specific directive/context lines instead of the entire file.

Comment thread scripts/update/php-update.sh
Comment on lines +135 to +137
sed -E -i \
-e "s|(unix:/run/php/)php${OLD_VER}-fpm(\.sock)|\1php${NEW_PHP_VER}-fpm\2|g" \
-e "s|(fastcgi_pass[[:space:]]+[^;]*php)${OLD_VER}(-fpm)|\1${NEW_PHP_VER}\2|g" \
Comment thread scripts/update/php-update.sh
if [[ -f "$config_file" ]]; then
for OLD_VER in "${MIGRATION_SOURCE_PHP_VERS[@]}"; do
sed -E -i "s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\1|g" "$config_file"
sed -E -i "/^[[:space:]]*fastcgi_pass[[:space:]]+/ s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\1|g" "$config_file"
Comment thread scripts/update/php-update.sh
echo "Updating phpSysInfo configuration..."
for OLD_VER in "${MIGRATION_SOURCE_PHP_VERS[@]}"; do
sed -E -i "s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\1|g" "/var/www/admin/tools/phpsysinfo/phpsysinfo.ini"
sed -E -i "s|^([[:space:]]*[[:alnum:]_.-]+[[:space:]]*=[[:space:]]*.*)php${OLD_VER}(-fpm)?|\1php${NEW_PHP_VER}\2|g" "/var/www/admin/tools/phpsysinfo/phpsysinfo.ini"
Comment thread scripts/update/php-update.sh
echo "Updating admin control panel API configuration..."
for OLD_VER in "${MIGRATION_SOURCE_PHP_VERS[@]}"; do
sed -E -i "s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\1|g" "/var/www/admin/control-panel/api.php"
sed -E -i "/(php-fpm|fastcgi_pass|sock(et)?|service)/ s|php${OLD_VER}(-fpm)?|php${NEW_PHP_VER}\1|g" "/var/www/admin/control-panel/api.php"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
+1 more reviewer
@PDowney PDowney PDowney approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@PDowney PDowney

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@PDowney