Mailing List Archive

Given the context of my Catalyst-users post yesterday:

http://lists.scsys.co.uk/pipermail/catalyst/2010-August/025730.html

At the end of this message is a WORKSFORME patch (what svn diff outputs) on the
Catalyst-Runtime trunk for which I haven't yet made a test.

I welcome feedback on it.

This change breaks 4-5 C-R tests, though I haven't looked at those yet and the
tests may be making false assumptions and need fixing themselves or my patch may
need more work.

The make-test for MyApp that had the problems now runs properly and still passes
its tests.

I intend to commit this patch myself (to set precedents and practice for being a
more regular committer eventually) in a new branch, after I have come up with a
test and looked at the failing tests, committing the new initially-failing test
first.

Then a core committer can merge it to trunk when it is deemed to be suitable.

The failing test summary report is:

Test Summary Report
-------------------
t/aggregate/live_engine_request_headers.t (Wstat: 256
Tests: 18 Failed: 1)
Failed test: 7
Non-zero exit status: 1
t/aggregate/unit_core_script_server.t (Wstat:
65280 Tests: 0 Failed: 0)
Non-zero exit status: 255
Parse errors: No plan found in TAP output
t/author/http-server.t (Wstat:
15360 Tests: 0 Failed: 0)
Non-zero exit status: 60
Parse errors: Bad plan. You planned 1 tests but ran 0.
t/dead_no_unknown_error.t (Wstat:
65280 Tests: 0 Failed: 0)
Non-zero exit status: 255
Parse errors: Bad plan. You planned 1 tests but ran 0.
t/unit_core_methodattributes_method_metaclass_on_subclasses.t (Wstat: 0
Tests: 2 Failed: 0)
TODO passed: 1
Files=131, Tests=2720, 252 wallclock secs ( 0.90 usr 0.59 sys + 210.54 cusr
8.47 csys = 220.50 CPU)
Result: FAIL
Failed 4/131 test programs. 1/2720 subtests failed.
make: *** [test_dynamic] Error 255

I welcome feedback.

Thank you. -- Darren Duncan



Index: lib/Catalyst/Engine/HTTP.pm
===================================================================
--- lib/Catalyst/Engine/HTTP.pm (revision 13513)
+++ lib/Catalyst/Engine/HTTP.pm (working copy)
@@ -97,6 +97,20 @@
*STDIN->blocking(1);
};

+=head2 $self->prepare_connection($c)
+
+=cut
+
+after prepare_connection => sub {
+ my ( $self, $c ) = @_;
+ # This test is inappropriate for CGI/FastCGI, which should just look
+ # for $ENV{HTTPS} being 'ON', since port 443 may not be HTTPS here.
+ # But it might be appropriate for HTTP-proxy which may lack HTTPS header.
+ if ( $ENV{SERVER_PORT} == 443 ) {
+ $request->secure(1);
+ }
+};
+
=head2 $self->prepare_read($c)

=cut
Index: lib/Catalyst/Engine/CGI.pm
===================================================================
--- lib/Catalyst/Engine/CGI.pm (revision 13513)
+++ lib/Catalyst/Engine/CGI.pm (working copy)
@@ -118,10 +118,13 @@
if ( $ENV{HTTPS} && uc( $ENV{HTTPS} ) eq 'ON' ) {
$request->secure(1);
}
+ # Declaring the request to be secure just because $ENV{SERVER_PORT} == 443
+ # is incorrect because port 443 may not always be running HTTPS; it might
+ # be running HTTP instead, for some reason.
+ # So testing that $ENV{HTTPS} exists and is 'ON' is the only reliable test
+ # for the CGI/FastCGI engines.
+ # But the HTTP-proxy engine might not see HTTPS and so it has the 443 test.

- if ( $ENV{SERVER_PORT} == 443 ) {
- $request->secure(1);
- }
binmode(STDOUT); # Ensure we are sending bytes.
}

@@ -215,9 +218,15 @@
my $uri_class = "URI::$scheme";

# HTTP_HOST will include the port even if it's 80/443
- $host =~ s/:(?:80|443)$//;
-
- if ( $port !~ /^(?:80|443)$/ && $host !~ /:/ ) {
+ if ($host =~ s/:(.*)$//) {
+ $port = $1; # since $ENV{SERVER_PORT} may be missing or wrong
+ }
+ if ($port == 80 and !$c->request->secure or $port == 443 and
$c->request->secure) {
+ # The actual port matches the default port for the security status,
+ # so leave $host without a :$port.
+ }
+ else {
+ # The actual port doesn't match the default port for the security status.
$host .= ":$port";
}


_______________________________________________
Catalyst-dev mailing list
Catalyst-dev@lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev