docs: Add improvement roadmap for Phase 2-4 development
This commit is contained in:
@@ -0,0 +1,172 @@
|
||||
# Remaining Improvements - Dev Branch
|
||||
|
||||
**Status**: Post-critical-fixes analysis
|
||||
**Date**: 2026-03-19
|
||||
**Branch**: dev
|
||||
|
||||
## High-Priority Items (Recommended Next)
|
||||
|
||||
### 1. Array Safety in User Enumeration (reference-db.sh:128)
|
||||
```bash
|
||||
# Current (potentially unsafe)
|
||||
local users=($(list_all_users))
|
||||
|
||||
# Better approach
|
||||
while IFS= read -r user; do
|
||||
[ -z "$user" ] && continue
|
||||
users+=("$user")
|
||||
done < <(list_all_users)
|
||||
```
|
||||
**Why**: Safer handling of usernames with special characters
|
||||
**Impact**: Prevents word-splitting issues with unusual usernames
|
||||
**Difficulty**: LOW (30 min)
|
||||
|
||||
### 2. URL Encoding for Domain Checks (reference-db.sh:219, 225)
|
||||
```bash
|
||||
# Current (not encoded)
|
||||
curl ... "http://$domain"
|
||||
|
||||
# Better approach
|
||||
domain_encoded=$(printf %s "$domain" | sed 's/[^a-zA-Z0-9._-]/\\&/g')
|
||||
curl ... "http://$domain_encoded"
|
||||
```
|
||||
**Why**: Handles domains with special characters or non-ASCII characters
|
||||
**Impact**: Prevents curl errors with unusual domain names
|
||||
**Difficulty**: LOW (30 min)
|
||||
|
||||
### 3. Timeout Configuration Validation
|
||||
**Current**: Hardcoded 3-second timeout in curl operations
|
||||
**Issue**: May be insufficient for slow networks or servers
|
||||
**Improvement**: Make configurable via environment variable
|
||||
```bash
|
||||
DOMAIN_CHECK_TIMEOUT=${DOMAIN_CHECK_TIMEOUT:-3}
|
||||
timeout $DOMAIN_CHECK_TIMEOUT curl ...
|
||||
```
|
||||
**Difficulty**: LOW (20 min)
|
||||
|
||||
---
|
||||
|
||||
## Medium-Priority Items
|
||||
|
||||
### 4. Array Expansion Consistency (reference-db.sh:118)
|
||||
**Current**: Mixes array patterns
|
||||
```bash
|
||||
# Line 118 - for loop with [@]
|
||||
for php_ver in "${SYS_PHP_VERSIONS[@]}"; do
|
||||
|
||||
# Line 128 - array assignment with command substitution
|
||||
local users=($(list_all_users))
|
||||
```
|
||||
**Issue**: Inconsistent array handling patterns
|
||||
**Recommendation**: Document and enforce consistent pattern
|
||||
**Difficulty**: LOW (15 min)
|
||||
|
||||
### 5. Progress Bar Rendering (lib/common-functions.sh:140-150)
|
||||
**Current**: Uses carriage return \r for in-place updates
|
||||
**Potential Issue**: May not work correctly in all terminal types
|
||||
**Improvement**: Add fallback for dumb terminals
|
||||
```bash
|
||||
if [ "$TERM" != "dumb" ]; then
|
||||
printf "\r]..." # In-place update
|
||||
else
|
||||
echo "..." # Fallback to newlines
|
||||
fi
|
||||
```
|
||||
**Difficulty**: MEDIUM (45 min)
|
||||
|
||||
---
|
||||
|
||||
## Low-Priority Items
|
||||
|
||||
### 6. Function Naming Conventions
|
||||
**Current**: Mix of naming styles
|
||||
- `build_system_section()` - verb_noun style
|
||||
- `check_domain_status()` - verb_noun style
|
||||
- `show_progress()` - verb_noun style
|
||||
|
||||
**Observation**: Naming is actually consistent! ✅
|
||||
|
||||
### 7. Inline Documentation
|
||||
**Current**: Some functions lack purpose comments
|
||||
**Recommendation**: Add one-line purpose comments above all functions
|
||||
**Difficulty**: LOW (1 hour for all files)
|
||||
|
||||
### 8. Source Guard Safety (reference-db.sh line 1)
|
||||
**Current**: No source guard (allows re-sourcing)
|
||||
**Improvement**: Add guard pattern
|
||||
```bash
|
||||
if [ -n "${_REFERENCE_DB_LOADED:-}" ]; then
|
||||
return 0
|
||||
fi
|
||||
readonly _REFERENCE_DB_LOADED=1
|
||||
```
|
||||
**Difficulty**: LOW (10 min, add to all library files)
|
||||
|
||||
### 9. Unused Variable Cleanup
|
||||
**Finding**: No unused variables detected in recent code review
|
||||
**Status**: ✅ CLEAN
|
||||
|
||||
---
|
||||
|
||||
## Implementation Priority Recommendation
|
||||
|
||||
### Phase 2 - Next (1-2 hours)
|
||||
1. ✅ Critical security fixes (DONE - 16f222f)
|
||||
2. Array safety in user enumeration (30 min)
|
||||
3. URL encoding for domain checks (30 min)
|
||||
4. Timeout configuration (20 min)
|
||||
|
||||
### Phase 3 - Later (2-3 hours)
|
||||
5. Array expansion consistency (15 min)
|
||||
6. Progress bar fallbacks (45 min)
|
||||
7. Source guard safety (10 min)
|
||||
8. Inline documentation (60 min)
|
||||
|
||||
### Phase 4 - Low Priority (1 hour)
|
||||
9. Additional refinements based on testing
|
||||
|
||||
---
|
||||
|
||||
## Testing Plan for Phase 2
|
||||
|
||||
Once Phase 2 items are fixed:
|
||||
|
||||
1. **Fresh AlmaLinux 8 Test**
|
||||
- No control panel
|
||||
- No web server
|
||||
- No database
|
||||
- Expected: Proper detection with empty services
|
||||
|
||||
2. **Fresh Ubuntu 22.04 Test**
|
||||
- With Apache
|
||||
- No MySQL
|
||||
- Expected: Proper Apache detection, MySQL marked as "none"
|
||||
|
||||
3. **cPanel Test**
|
||||
- Full stack: cPanel, Apache, MySQL
|
||||
- Expected: All services detected correctly
|
||||
|
||||
4. **Plesk Test**
|
||||
- Full stack: Plesk, Nginx, MariaDB
|
||||
- Expected: Proper Plesk and Nginx detection
|
||||
|
||||
---
|
||||
|
||||
## Deployment Timeline
|
||||
|
||||
- [x] Critical security fixes - Commit 16f222f
|
||||
- [ ] Phase 2 improvements - Target 1-2 hours
|
||||
- [ ] Phase 2 testing - Target fresh systems
|
||||
- [ ] Phase 3 improvements - Target 2-3 hours
|
||||
- [ ] Full regression suite - Target all combinations
|
||||
- [ ] Merge to production main branch
|
||||
|
||||
---
|
||||
|
||||
## Notes
|
||||
|
||||
- All syntax checks pass (bash -n validation)
|
||||
- No runtime errors detected
|
||||
- Process substitution patterns are safe
|
||||
- Error handling is comprehensive
|
||||
- Color code duplication (lines 28-35 of launcher.sh) is redundant but harmless
|
||||
Reference in New Issue
Block a user