michael/webmin
1
0
Fork 0
mirror of https://github.com/webmin/webmin.git synced 2026-09-22 13:30:43 +01:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

Merge pull request #2847 from webmin/dev/preserve-apache-comments

Browse Source 
Add Apache comment preservation
This commit is contained in:
Jamie Cameron
2026-09-19 17:33:05 -07:00
committed by GitHub
parent c5cf9242c7 efe1dddca8
commit 1b029fb2c3
2 changed files with 281 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

94
apache/apache-lib.pl
View File

@@ -126,9 +126,10 @@ if (&read_file($site_file, \%site)) {
# value - Value (possibly with spaces)
# members - For type 1, a reference to the array of members
# indent - Number of spaces before the name
# comments - Full comment lines immediately before this directive
sub parse_config_file
{
local($fh, @rv, $line, %dummy);
local($fh, @rv, $line, %dummy, @comments);
$fh = $_[0];
$dummy{'line'} = $dummy{'eline'} = $_[1]-1;
$dummy{'file'} = $_[2];
@@ -146,8 +147,13 @@ foreach my $d (&get_httpd_defines()) {
}
while($line = <$fh>) {
$line =~ s/\r|\n//g;
$line =~ s/^\s*#.*$//g;
if ($line =~ /^\s*<\/(\S+)\s*(.*)>/) {
if ($line =~ /^\s*#/) {
# Attach comments to the next directive without reusing the
# dummy entry that marks the start of this block
push(@comments, $line);
$_[1]++;
}
elsif ($line =~ /^\s*<\/(\S+)\s*(.*)>/) {
# end of a container directive. This can only happen in a
# recursive call to this function
$_[1]++;
@@ -178,10 +184,13 @@ while($line = <$fh>) {
$not && !$httpd_modules{$mpmmod}
) {
# use the directives..
push(@rv, { 'line', $oldline,
'eline', $oldline,
'file', $_[2],
'name', "<IfModule $not$mod>" });
local %open = ( 'line', $oldline,
'eline', $oldline,
'file', $_[2],
'name', "<IfModule $not$mod>" );
$open{'comments'} = [ @comments ] if (@comments);
@comments = ( );
push(@rv, \%open);
push(@rv, @dirs);
push(@rv, { 'line', $_[1]-1,
'eline', $_[1]-1,
@@ -199,10 +208,13 @@ while($line = <$fh>) {
if (!$not && defined($defs{$def}) ||
$not && !defined($defs{$def})) {
# use the directives..
push(@rv, { 'line', $oldline,
'eline', $oldline,
'file', $_[2],
'name', "<IfDefine $not$def>" });
local %open = ( 'line', $oldline,
'eline', $oldline,
'file', $_[2],
'name', "<IfDefine $not$def>" );
$open{'comments'} = [ @comments ] if (@comments);
@comments = ( );
push(@rv, \%open);
push(@rv, @dirs);
push(@rv, { 'line', $_[1]-1,
'eline', $_[1]-1,
@@ -247,10 +259,13 @@ while($line = <$fh>) {
$match = !$match if ($not);
if ($match) {
# use the directives..
push(@rv, { 'line', $oldline,
'eline', $oldline,
'file', $_[2],
'name', "<IfVersion $not$op $ver>" });
local %open = ( 'line', $oldline,
'eline', $oldline,
'file', $_[2],
'name', "<IfVersion $not$op $ver>" );
$open{'comments'} = [ @comments ] if (@comments);
@comments = ( );
push(@rv, \%open);
push(@rv, @dirs);
push(@rv, { 'line', $_[1]-1,
'eline', $_[1]-1,
@@ -276,6 +291,8 @@ while($line = <$fh>) {
$dir{'eline'} = $_[1]-1;
$indent =~ s/\t/ /g;
$dir{'indent'} = length($indent);
$dir{'comments'} = [ @comments ] if (@comments);
@comments = ( );
push(@rv, \%dir);
}
elsif ($line =~ /^(\s*)(\S+)\s*(.*)$/) {
@@ -309,11 +326,15 @@ while($line = <$fh>) {
}
}
$dir{'words'} = &wsplit($dir{'value'});
$dir{'comments'} = [ @comments ] if (@comments);
@comments = ( );
push(@rv, \%dir);
$_[1]++;
}
else {
# blank or comment line
# Keep blank lines that separate a pending comment block from
# the directive it describes
push(@comments, $line) if (@comments);
$_[1]++;
}
}
@@ -636,9 +657,12 @@ for($i=0; $i<@old || $i<@{$_[1]}; $i++) {
push(@files, $old[$i]->{'file'});
$idx = &indexof($old[$i], @{$_[2]});
splice(@{$_[2]}, $idx, 1);
$len = $old[$i]->{'eline'} - $old[$i]->{'line'} + 1;
splice(@$lref, $old[$i]->{'line'}, $len);
&renumber($_[3], $old[$i]->{'line'}, $old[$i]->{'file'}, -$len);
local $comments = $old[$i]->{'comments'} ?
scalar(@{$old[$i]->{'comments'}}) : 0;
local $start = $old[$i]->{'line'}-$comments;
$len = $old[$i]->{'eline'}-$start+1;
splice(@$lref, $start, $len);
&renumber($_[3], $start, $old[$i]->{'file'}, -$len);
}
else {
# just changing the value
@@ -673,9 +697,16 @@ return if (!$olddir && !$newdir); # Nothing to do
local $file = $olddir ? $olddir->{'file'} :
$newdir->{'file'} ? $newdir->{'file'} : $pconf->[0]->{'file'};
local $lref = &read_file_lines($file);
local $oldlen = $olddir ? $olddir->{'eline'}-$olddir->{'line'}+1 : undef;
local $oldcomments = $olddir && $olddir->{'comments'} ?
scalar(@{$olddir->{'comments'}}) : 0;
local $oldline = $olddir ? $olddir->{'line'}-$oldcomments : undef;
local $oldlen = $olddir ? $olddir->{'eline'}-$oldline+1 : undef;
local @newlines;
if ($newdir) {
if ($olddir && !exists($newdir->{'comments'}) && $oldcomments) {
# A replacement inherits the comments attached to the old block
$newdir->{'comments'} = [ @{$olddir->{'comments'}} ];
}
my $isrc = $olddir ? $olddir :
@$pconf ? $pconf->[0] : undef;
if ($isrc) {
@@ -689,7 +720,9 @@ if ($olddir && $newdir) {
$newdir->{'words'} = &wsplit($newdir->{'value'});
if ($first) {
# Just changing first and last line, like virtualhost IP
$lref->[$olddir->{'line'}] = $newlines[0];
local $comments = $newdir->{'comments'} ?
scalar(@{$newdir->{'comments'}}) : 0;
$lref->[$olddir->{'line'}] = $newlines[$comments];
$lref->[$olddir->{'eline'}] = $newlines[$#newlines];
$olddir->{'name'} = $newdir->{'name'};
$olddir->{'value'} = $newdir->{'value'};
@@ -702,9 +735,11 @@ if ($olddir && $newdir) {
local $idx = &indexof($olddir, @$pconf);
$pconf->[$idx] = $newdir if ($idx >= 0);
$newdir->{'file'} = $olddir->{'file'};
$newdir->{'line'} = $olddir->{'line'};
$newdir->{'eline'} = $olddir->{'line'}+scalar(@newlines)-1;
splice(@$lref, $olddir->{'line'}, $oldlen, @newlines);
local $comments = $newdir->{'comments'} ?
scalar(@{$newdir->{'comments'}}) : 0;
$newdir->{'line'} = $oldline+$comments;
$newdir->{'eline'} = $oldline+scalar(@newlines)-1;
splice(@$lref, $oldline, $oldlen, @newlines);
# Update sub-directive lines and files too
if ($newdir->{'type'}) {
@@ -716,10 +751,10 @@ if ($olddir && $newdir) {
}
elsif ($olddir && !$newdir) {
# Remove
splice(@$lref, $olddir->{'line'}, $oldlen);
splice(@$lref, $oldline, $oldlen);
local $idx = &indexof($olddir, @$pconf);
splice(@$pconf, $idx, 1) if ($idx >= 0);
&renumber($conf, $olddir->{'line'}, $olddir->{'file'}, -$oldlen);
&renumber($conf, $oldline, $olddir->{'file'}, -$oldlen);
}
elsif (!$olddir && $newdir) {
# Add to file, at end of specific file or parent section
@@ -739,7 +774,9 @@ elsif (!$olddir && $newdir) {
}
$newdir->{'words'} = &wsplit($newdir->{'value'});
$newdir->{'file'} = $file;
$newdir->{'line'} = $addline;
local $comments = $newdir->{'comments'} ?
scalar(@{$newdir->{'comments'}}) : 0;
$newdir->{'line'} = $addline+$comments;
$newdir->{'eline'} = $addline + scalar(@newlines) - 1;
&renumber($conf, $addline, $file, scalar(@newlines));
splice(@$pconf, $addpos, 0, $newdir);
@@ -776,6 +813,8 @@ sub recursive_set_lines_files
{
my ($dirs, $line, $file) = @_;
foreach my $dir (@$dirs) {
# A directive's line follows any comments attached to it
$line += scalar(@{$dir->{'comments'}}) if ($dir->{'comments'});
$dir->{'line'} = $line;
$dir->{'file'} = $file;
if ($dir->{'type'}) {
@@ -1930,6 +1969,7 @@ sub directive_lines
my @rv;
foreach my $d (@_) {
next if ($d->{'name'} eq 'dummy');
push(@rv, @{$d->{'comments'}}) if ($d->{'comments'});
my $indent = (" " x $d->{'indent'});
if ($d->{'type'}) {
push(@rv, $indent."<$d->{'name'} $d->{'value'}>");

214
apache/t/directive-comments.t Normal file
View File

@@ -0,0 +1,214 @@
#!/usr/bin/perl
# Full-line comments must survive Apache block rewrites.
use strict;
use warnings;
use Test::More;
use File::Basename qw(dirname);
use File::Path qw(make_path);
use File::Spec;
use File::Temp qw(tempdir);
use Cwd qw(abs_path);
my $root = abs_path(File::Spec->catdir(dirname(__FILE__), '..', '..'));
my $tmp = abs_path(tempdir(CLEANUP => 1));
my $webmin_config = File::Spec->catdir($tmp, 'webmin-config');
my $webmin_var = File::Spec->catdir($tmp, 'webmin-var');
my $apache_root = File::Spec->catdir($tmp, 'apache2');
my $apache_conf = File::Spec->catfile($apache_root, 'apache2.conf');
make_path($webmin_config, $webmin_var, "$webmin_config/apache",
"$webmin_var/apache", $apache_root);
sub write_text
{
my ($file, $text) = @_;
open(my $fh, '>', $file) || die "Failed to write $file: $!";
print $fh $text;
close($fh) || die "Failed to close $file: $!";
}
sub read_text
{
my ($file) = @_;
open(my $fh, '<', $file) || die "Failed to read $file: $!";
local $/ = undef;
my $text = <$fh>;
close($fh) || die "Failed to close $file: $!";
return $text;
}
# Load the Apache module with an isolated Webmin configuration.
write_text(File::Spec->catfile($webmin_config, 'config'),
"os_type=debian-linux\n".
"os_version=12\n");
write_text(File::Spec->catfile($webmin_config, 'miniserv.conf'),
"root=$root\n");
write_text(File::Spec->catfile($webmin_config, 'apache', 'config'),
"httpd_dir=$apache_root\n".
"httpd_path=/bin/true\n".
"httpd_conf=$apache_conf\n".
"apachectl_path=/bin/true\n".
"httpd_version=2.4.57\n");
write_text($apache_conf, "Listen 80\n");
$ENV{'WEBMIN_CONFIG'} = $webmin_config;
$ENV{'WEBMIN_VAR'} = $webmin_var;
$ENV{'FOREIGN_MODULE_NAME'} = 'apache';
$ENV{'FOREIGN_ROOT_DIRECTORY'} = $root;
$ENV{'REMOTE_USER'} = 'root';
unshift(@INC, $root);
require File::Spec->catfile($root, 'apache', 'apache-lib.pl');
# Comments at multiple indentation levels must round-trip with the directives.
my $roundtrip = File::Spec->catfile($tmp, 'comments.conf');
my $roundtrip_text =
"# outer comment\n".
"<Directory /srv/example>\n".
" # nested comment\n".
"\n".
" Require all granted\n".
"</Directory>\n";
write_text($roundtrip, $roundtrip_text);
open(my $fh, '<', $roundtrip) || die "Failed to read $roundtrip: $!";
my $line = 0;
my @roundtrip_config = main::parse_config_file($fh, $line, $roundtrip);
close($fh) || die "Failed to close $roundtrip: $!";
my ($directory) =
main::find_directive_struct('Directory', \@roundtrip_config);
my ($require) =
main::find_directive_struct('Require', $directory->{'members'});
is_deeply($directory->{'comments'}, [ '# outer comment' ],
'outer comment is attached to the directory');
is_deeply($require->{'comments'}, [ ' # nested comment', '' ],
'nested comment block is attached to the following directive');
ok(!$roundtrip_config[0]->{'comments'} &&
!$directory->{'members'}->[0]->{'comments'},
'dummy block markers do not store comments');
is(join("\n", main::directive_lines(@roundtrip_config))."\n",
$roundtrip_text, 'comments survive parsing and serialization');
# Rewriting and removing virtual hosts must handle attached comments with them.
my $vhosts_file = File::Spec->catfile($tmp, 'vhosts.conf');
my $first_vhost =
"# first virtual host\n".
"<VirtualHost *:80>\n".
" # first comment\n".
"\n".
" ServerName first.example\n".
"</VirtualHost>\n";
my $second_vhost =
"# second virtual host\n".
"<VirtualHost *:443>\n".
" # second comment\n".
" ServerName second.example\n".
"</VirtualHost>\n";
write_text($vhosts_file, $first_vhost.$second_vhost);
open($fh, '<', $vhosts_file) || die "Failed to read $vhosts_file: $!";
$line = 0;
my @vhost_config = main::parse_config_file($fh, $line, $vhosts_file);
close($fh) || die "Failed to close $vhosts_file: $!";
my @vhosts = main::find_directive_struct('VirtualHost', \@vhost_config);
main::recursive_set_lines_files(\@vhost_config, 0, $vhosts_file);
is($vhosts[0]->{'line'}, 1, 'block line follows its attached comment');
is($vhosts[0]->{'eline'}, 5, 'member comment is included in block line count');
is($vhosts[1]->{'line'}, 7, 'next block follows its attached comment');
my %replacement = %{$vhosts[0]};
delete($replacement{'comments'});
$replacement{'value'} = '*:8080';
my $rewritten_first = $first_vhost;
$rewritten_first =~ s/\*:80>/*:8080>/;
main::save_directive_struct($vhosts[0], \%replacement,
\@vhost_config, \@vhost_config);
main::flush_file_lines($vhosts_file);
is(read_text($vhosts_file), $rewritten_first.$second_vhost,
'replacement block preserves surrounding comments');
is_deeply($replacement{'comments'}, [ '# first virtual host' ],
'replacement block inherits attached comments');
main::save_directive_struct($vhosts[1], undef,
\@vhost_config, \@vhost_config);
main::flush_file_lines($vhosts_file);
is(read_text($vhosts_file),
$rewritten_first,
'deleting a block removes its attached comment');
# Normal directive edits preserve comments, while deletion removes their owner.
my $directives_file = File::Spec->catfile($tmp, 'directives.conf');
my $directives_text =
"# listen comment\n".
"Listen 80\n".
"# server name comment\n".
"ServerName old.example\n";
write_text($directives_file, $directives_text);
open($fh, '<', $directives_file) ||
die "Failed to read $directives_file: $!";
$line = 0;
my @directive_config =
main::parse_config_file($fh, $line, $directives_file);
close($fh) || die "Failed to close $directives_file: $!";
main::save_directive('ServerName', [ 'new.example' ],
\@directive_config, \@directive_config);
main::flush_file_lines($directives_file);
my $updated_directives =
"# listen comment\n".
"Listen 80\n".
"# server name comment\n".
"ServerName new.example\n";
is(read_text($directives_file), $updated_directives,
'comments survive a normal directive edit');
main::save_directive('Listen', [ ], \@directive_config, \@directive_config);
main::flush_file_lines($directives_file);
my $deleted_directive =
"# server name comment\n".
"ServerName new.example\n";
is(read_text($directives_file), $deleted_directive,
'deleting a directive removes its attached comment');
open($fh, '<', $directives_file) ||
die "Failed to read $directives_file: $!";
$line = 0;
@directive_config = main::parse_config_file($fh, $line, $directives_file);
close($fh) || die "Failed to close $directives_file: $!";
my ($servername) =
main::find_directive_struct('ServerName', \@directive_config);
is_deeply($servername->{'comments'},
[ '# server name comment' ],
'remaining comment stays attached to its directive');
# Nested block positions must include comments at every level.
my $inner = {
'name' => 'Files', 'value' => '*.php', 'type' => 1,
'comments' => [ ' # files comment' ],
'members' => [
{ 'name' => 'dummy', 'type' => 0 },
{ 'name' => 'Require', 'value' => 'all denied',
'comments' => [ ' # access comment' ] },
],
};
my $outer = {
'name' => 'Directory', 'value' => '/srv/example', 'type' => 1,
'members' => [
{ 'name' => 'dummy', 'type' => 0 },
$inner,
{ 'name' => 'Options', 'value' => 'Indexes' },
],
};
my @nested = ($outer);
my @nested_lines = main::directive_lines(@nested);
my $next_line = main::recursive_set_lines_files(\@nested, 20, $vhosts_file);
is($inner->{'line'}, 22, 'nested block line follows its comment');
is($inner->{'members'}->[1]->{'line'}, 24,
'nested member line follows its comment');
is($outer->{'members'}->[2]->{'line'}, 26,
'directive after nested block has the correct line');
is($outer->{'eline'}, 27, 'outer block ends after all nested lines');
is($next_line, 20 + scalar(@nested_lines),
'line count matches nested serialized output');
done_testing();