* [PATCH 1/2] remove direct CGI.pm support
2016-02-25 3:10 [PATCH 0/2] remove CGI.pm support Eric Wong
@ 2016-02-25 3:10 ` Eric Wong
2016-02-25 3:10 ` [PATCH 2/2] www: make interface more OO Eric Wong
1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2016-02-25 3:10 UTC (permalink / raw)
To: meta
Relying on Plack::Handler::CGI is much easier for long-term
maintenance and development.
Nowadays, we even include our own httpd implementation to
facilitate easier deployment with PSGI/Plack.
---
INSTALL | 5 +--
Makefile.PL | 2 +-
lib/PublicInbox/ExtMsg.pm | 11 +-----
lib/PublicInbox/Feed.pm | 9 +----
lib/PublicInbox/GitHTTPBackend.pm | 6 +--
lib/PublicInbox/WWW.pm | 24 ++----------
public-inbox.cgi | 77 +++++++++++++--------------------------
7 files changed, 36 insertions(+), 98 deletions(-)
diff --git a/INSTALL b/INSTALL
index 538a95d..54c344c 100644
--- a/INSTALL
+++ b/INSTALL
@@ -39,7 +39,7 @@ Requirements (server MDA)
Optional modules:
- - CGI[1] perl-modules[2]
+ - Plack[1] libplack-perl
- Mail::Thread (2.5+)[1] libmail-thread-perl
- URI::Escape[1] liburi-perl
- Search::Xapian[3] libsearch-xapian-perl
@@ -49,9 +49,6 @@ Optional modules:
- Danga::Socket[4] libdanga-socket-perl
[1] - Only required for serving/generating Atom and HTML pages.
-[2] - Keep in mind this will be split into a separate Debian package
- when CGI.pm is dropped from the Perl standard library.
- Plack/PSGI and mod_perl2 are both supported.
[3] - Optional for HTML web interface and NNTP server
[4] - Optional for NNTP server
diff --git a/Makefile.PL b/Makefile.PL
index 5eec5ef..621baf6 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -17,7 +17,6 @@ WriteMakefile(
# note: we use ssoma(1) and spamc(1), NOT the Perl modules
# We also depend on git through ssoma.
# Keep this sorted and synced to the INSTALL document
- 'CGI' => 0,
'Date::Parse' => 0,
'Email::Address' => 0,
'Email::Filter' => 0,
@@ -27,6 +26,7 @@ WriteMakefile(
'File::Path::Expand' => 0,
'IPC::Run' => 0,
'Mail::Thread' => '2.5', # 2.5+ needed for Email::Simple compat
+ 'Plack' => 0,
'URI::Escape' => 0,
# We have more test dependencies, but do not force
# users to install them
diff --git a/lib/PublicInbox/ExtMsg.pm b/lib/PublicInbox/ExtMsg.pm
index d89a7e3..cf9b6d5 100644
--- a/lib/PublicInbox/ExtMsg.pm
+++ b/lib/PublicInbox/ExtMsg.pm
@@ -86,15 +86,7 @@ sub ext_msg {
eval { require PublicInbox::Msgmap };
my $have_mm = $@ ? 0 : 1;
my $cgi = $ctx->{cgi};
- my $base_url;
- my $scheme;
- if (ref($cgi) eq 'CGI') {
- $base_url = $cgi->url(-base) . '/';
- $scheme = $cgi->protocol;
- } else { # Plack::Request
- $base_url = $cgi->base->as_string;
- $scheme = $cgi->env->{'psgi.url_scheme'};
- }
+ my $base_url = $cgi->base->as_string;
if ($have_mm) {
my $tmp_mid = $mid;
my $url;
@@ -145,6 +137,7 @@ again:
if (@EXT_URL && index($mid, '@') >= 0) {
$code = 300;
$s .= "\nPerhaps try an external site:\n\n";
+ my $scheme = $cgi->scheme;
foreach my $u (@EXT_URL) {
$u = "$scheme:$u" if $u =~ m!\A//!;
my $r = sprintf($u, $href);
diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index a0aa62a..a5828a8 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -284,14 +284,7 @@ sub get_feedopts {
my $url_base;
if ($cgi) {
- my $base;
- if (ref($cgi) eq 'CGI') {
- $base = $cgi->url(-base);
- } else { # Plack::Request
- $base = $cgi->base->as_string;
- $base =~ s!/\z!!;
- }
- $url_base = "$base/$listname";
+ $url_base = $cgi->base->as_string . $listname;
if (my $mid = $ctx->{mid}) { # per-thread feed:
$rv{atomurl} = "$url_base/$mid/t.atom";
} else {
diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index 71b7a8f..562c290 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -55,7 +55,7 @@ sub serve {
my $len = $size;
my @h;
- my $env = $cgi->{env} || \%ENV;
+ my $env = $cgi->{env};
my $range = $env->{HTTP_RANGE};
if (defined $range && $range =~ /\bbytes=(\d*)-(\d*)\z/) {
($code, $len) = prepare_range($cgi, $in, \@h, $1, $2, $size);
@@ -117,9 +117,7 @@ sub prepare_range {
push @$h, "bytes $beg-$end/$size";
# FIXME: Plack::Middleware::Deflater bug?
- if (my $env = $cgi->{env}) {
- $env->{'psgix.no-compress'} = 1;
- }
+ $cgi->{env}->{'psgix.no-compress'} = 1;
}
}
($code, $len);
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 1f28df2..651c19e 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -223,12 +223,6 @@ sub get_thread {
PublicInbox::View::thread_html($ctx, $foot, $srch);
}
-sub self_url {
- my ($cgi) = @_;
- # Plack::Request
- ref($cgi) eq 'CGI' ? $cgi->self_url : $cgi->uri->as_string;
-}
-
sub ctx_get {
my ($ctx, $key) = @_;
my $val = $ctx->{$key};
@@ -258,9 +252,7 @@ sub footer {
my $urls = try_cat("$git_dir/cloneurl");
my @urls = split(/\r?\n/, $urls || '');
my %seen = map { $_ => 1 } @urls;
- my $cgi = $ctx->{cgi};
- my $http = (ref($cgi) eq 'CGI') ? $cgi->url(-base) . "/$listname" :
- $cgi->base->as_string . $listname;
+ my $http = $ctx->{cgi}->base->as_string . $listname;
$seen{$http} or unshift @urls, $http;
if (scalar(@urls) == 1) {
$urls = "URL for <a\nhref=\"" . SSOMA_URL .
@@ -325,7 +317,7 @@ sub get_thread_mbox {
sub get_thread_atom {
my ($ctx) = @_;
searcher($ctx) or return need_search($ctx);
- $ctx->{self_url} = self_url($ctx->{cgi});
+ $ctx->{self_url} = $ctx->{cgi}->uri->as_string;
require PublicInbox::Feed;
PublicInbox::Feed::generate_thread_atom($ctx);
}
@@ -390,16 +382,8 @@ sub r301 {
my ($ctx, $listname, $mid, $suffix) = @_;
my $cgi = $ctx->{cgi};
my $url;
- my $qs;
- if (ref($cgi) eq 'CGI') {
- $url = $cgi->url(-base) . '/';
- $qs = $cgi->query_string;
- } else { # Plack::Request
- $url = $cgi->base->as_string;
- $qs = $cgi->env->{QUERY_STRING};
- }
-
- $url .= $listname . '/';
+ my $qs = $cgi->env->{QUERY_STRING};
+ $url = $cgi->base->as_string . $listname . '/';
$url .= (uri_escape_utf8($mid) . '/') if (defined $mid);
$url .= $suffix if (defined $suffix);
$url .= "?$qs" if $qs ne '';
diff --git a/public-inbox.cgi b/public-inbox.cgi
index 4b74a62..e73e23c 100755
--- a/public-inbox.cgi
+++ b/public-inbox.cgi
@@ -1,62 +1,35 @@
#!/usr/bin/perl -w
-# Copyright (C) 2014-2015 all contributors <meta@public-inbox.org>
-# License: AGPLv3 or later (https://www.gnu.org/licenses/agpl-3.0.txt)
+# Copyright (C) 2014-2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ or later <https://www.gnu.org/licenses/agpl-3.0.txt>
#
# Enables using PublicInbox::WWW as a CGI script
use strict;
use warnings;
-use IO::Handle;
+use Plack::Loader;
+use Plack::Builder;
+use Plack::Request;
+use Plack::Handler::CGI;
use PublicInbox::WWW;
-use CGI qw/-nosticky/;
-our $NO_SCRIPT_NAME;
-our %HTTP_CODES;
-BEGIN {
- $NO_SCRIPT_NAME = 1 if $ENV{NO_SCRIPT_NAME};
- if ($ENV{MOD_PERL}) {
- CGI->compile;
- PublicInbox::WWW->preload;
- }
-
- %HTTP_CODES = (
- 200 => 'OK',
- 300 => 'Multiple Choices',
- 301 => 'Moved Permanently',
- 302 => 'Found',
- 404 => 'Not Found',
- 405 => 'Method Not Allowed',
- 501 => 'Not Implemented',
- );
-}
-
-# some servers (Ruby webrick) include scheme://host[:port] here,
-# which confuses CGI.pm when generating self_url.
-# RFC 3875 does not mention REQUEST_URI at all,
-# so nuke it since CGI.pm functions without it.
-delete $ENV{REQUEST_URI};
-$ENV{SCRIPT_NAME} = '' if $NO_SCRIPT_NAME;
-my $req = CGI->new;
-my $ret = PublicInbox::WWW::run($req, $req->request_method);
+BEGIN { PublicInbox::WWW->preload if $ENV{MOD_PERL} }
-my $out = select;
-$out->binmode;
-
-if (ref($ret) eq 'CODE') {
- $ret->(*dump_header);
-} else {
- my ($status, $headers, $body) = @$ret;
+my $have_deflater = eval { require Plack::Middleware::Deflater; 1 };
+my $app = builder {
+ if ($have_deflater) {
+ enable 'Deflater',
+ content_type => [ 'text/html', 'text/plain',
+ 'application/atom+xml' ];
+ }
- dump_header([$status, $headers])->write($body->[0]);
-}
+ # Enable to ensure redirects and Atom feed URLs are generated
+ # properly when running behind a reverse proxy server which
+ # sets X-Forwarded-For and X-Forwarded-Proto request headers.
+ # See Plack::Middleware::ReverseProxy documentation for details
+ # enable 'ReverseProxy';
-sub dump_header {
- my ($res) = @_;
- my $fh = select;
- my ($status, $headers) = @$res;
- $fh->write("Status: $status $HTTP_CODES{$status}\r\n");
- my @tmp = @$headers;
- while (my ($k, $v) = splice(@tmp, 0, 2)) {
- $fh->write("$k: $v\r\n");
+ enable 'Head';
+ sub {
+ my $req = Plack::Request->new(@_);
+ PublicInbox::WWW::run($req, $req->method);
}
- $fh->write("\r\n");
- $fh;
-}
+};
+Plack::Handler::CGI->new->run($app);
--
EW
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] www: make interface more OO
2016-02-25 3:10 [PATCH 0/2] remove CGI.pm support Eric Wong
2016-02-25 3:10 ` [PATCH 1/2] remove direct " Eric Wong
@ 2016-02-25 3:10 ` Eric Wong
1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2016-02-25 3:10 UTC (permalink / raw)
To: meta
This allows multiple instances the WWW app from
running within the same process space
---
examples/public-inbox.psgi | 7 ++-----
lib/PublicInbox/WWW.pm | 24 ++++++++++++++++++------
public-inbox-httpd | 7 ++-----
public-inbox.cgi | 7 ++-----
4 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/examples/public-inbox.psgi b/examples/public-inbox.psgi
index c4a8903..e3e4215 100644
--- a/examples/public-inbox.psgi
+++ b/examples/public-inbox.psgi
@@ -10,7 +10,7 @@ PublicInbox::WWW->preload;
use Plack::Request;
use Plack::Builder;
my $have_deflater = eval { require Plack::Middleware::Deflater; 1 };
-
+my $www = PublicInbox::WWW->new;
builder {
enable 'Chunked';
if ($have_deflater) {
@@ -25,8 +25,5 @@ builder {
# See Plack::Middleware::ReverseProxy documentation for details
# enable 'ReverseProxy';
enable 'Head';
- sub {
- my $req = Plack::Request->new(@_);
- PublicInbox::WWW::run($req, $req->method);
- }
+ sub { $www->call(@_) };
}
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 651c19e..e87e559 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -22,14 +22,26 @@ use PublicInbox::GitHTTPBackend;
our $LISTNAME_RE = qr!\A/([\w\.\-]+)!;
our $MID_RE = qr!([^/]+)!;
our $END_RE = qr!(f/|T/|t/|R/|t\.mbox(?:\.gz)?|t\.atom|raw|)!;
-our $pi_config;
-sub run {
- my ($cgi, $method) = @_;
+sub new {
+ my ($class, $pi_config) = @_;
$pi_config ||= PublicInbox::Config->new;
- my $ctx = { cgi => $cgi, pi_config => $pi_config };
+ bless { pi_config => $pi_config }, $class;
+}
+
+# backwards compatibility, do not use
+sub run {
+ my ($req, $method) = @_;
+ PublicInbox::WWW->new->call($req->env);
+}
+
+sub call {
+ my ($self, $env) = @_;
+ my $cgi = Plack::Request->new($env);
+ my $ctx = { cgi => $cgi, pi_config => $self->{pi_config} };
my $path_info = $cgi->path_info;
+ my $method = $cgi->method;
if ($method eq 'POST' &&
$path_info =~ m!$LISTNAME_RE/(git-upload-pack)\z!) {
my $path = $2;
@@ -107,7 +119,7 @@ sub r { [ $_[0], ['Content-Type' => 'text/plain'], [ join(' ', @_, "\n") ] ] }
# returns undef if valid, array ref response if invalid
sub invalid_list {
my ($ctx, $listname) = @_;
- my $git_dir = $pi_config->get($listname, "mainrepo");
+ my $git_dir = $ctx->{pi_config}->get($listname, "mainrepo");
if (defined $git_dir) {
$ctx->{git_dir} = $git_dir;
$ctx->{git} = PublicInbox::Git->new($git_dir);
@@ -264,7 +276,7 @@ sub footer {
join("\n", map { "\t$_" } @urls);
}
- my $addr = $pi_config->get($listname, 'address');
+ my $addr = $ctx->{pi_config}->get($listname, 'address');
if (ref($addr) eq 'ARRAY') {
$addr = $addr->[0]; # first address is primary
}
diff --git a/public-inbox-httpd b/public-inbox-httpd
index 6436bd7..3635c9a 100644
--- a/public-inbox-httpd
+++ b/public-inbox-httpd
@@ -24,6 +24,7 @@ my $refresh = sub {
"$0 runs in /, command-line paths must be absolute\n";
}
} else {
+ my $www = PublicInbox::WWW->new;
$app = eval {
my $deflate_types = eval {
require Plack::Middleware::Deflater;
@@ -37,11 +38,7 @@ my $refresh = sub {
content_type => $deflate_types
}
enable 'Head';
- sub {
- my $req = Plack::Request->new(@_);
- PublicInbox::WWW::run($req,
- $req->method);
- };
+ sub { $www->call(@_) };
};
};
}
diff --git a/public-inbox.cgi b/public-inbox.cgi
index e73e23c..ee9510c 100755
--- a/public-inbox.cgi
+++ b/public-inbox.cgi
@@ -11,7 +11,7 @@ use Plack::Request;
use Plack::Handler::CGI;
use PublicInbox::WWW;
BEGIN { PublicInbox::WWW->preload if $ENV{MOD_PERL} }
-
+my $www = PublicInbox::WWW->new;
my $have_deflater = eval { require Plack::Middleware::Deflater; 1 };
my $app = builder {
if ($have_deflater) {
@@ -27,9 +27,6 @@ my $app = builder {
# enable 'ReverseProxy';
enable 'Head';
- sub {
- my $req = Plack::Request->new(@_);
- PublicInbox::WWW::run($req, $req->method);
- }
+ sub { $www->call(@_) };
};
Plack::Handler::CGI->new->run($app);
--
EW
^ permalink raw reply related [flat|nested] 3+ messages in thread