Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Something subtle has changed in .pairup, this fixes it #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jonathanstowe
Copy link
Contributor

Not quite sure what has changed in .pairup but it was getting

P6opaque: no such attribute '$!value' in type Pair when trying to get a value
  in block <unit> at -e line 1

when it encountered the pairs in the headers. This papers over that for the time being.

@skaji
Copy link
Collaborator

skaji commented Aug 27, 2017

Could you give us a p6w app that emits an error in pairup?

@jonathanstowe
Copy link
Contributor Author

It fails the t/Crust-Middleware/lint.t 👍

        not ok 1 - 
        # Failed test at t/Crust-Middleware/lint.t line 113
        # The number of response headers needs to be even, not odd(Content-Typetext/plain Content-Length	123)
        1..1
        # Looks like you failed 1 test of 1
    not ok 1 - Should works fine
    # Failed test 'Should works fine'
    # at t/Crust-Middleware/lint.t line 104

But the actual reason isn't evident because the CATCH in the block there, if the exception is printed in the default case there it becomes evident that every invocation of validate-ret gives rise to the same error:

P6opaque: no such attribute '$!value' in type Pair when trying to get a value
  in sub validate-ret at /home/jonathan/devel/perl6/3rdparty-modules/p6-Crust/lib/Crust/Middleware/Lint.pm6 (Crust::Middleware::Lint) line 67
  in block  at /home/jonathan/devel/perl6/3rdparty-modules/p6-Crust/lib/Crust/Middleware/Lint.pm6 (Crust::Middleware::Lint) line 113

This only gives rise to one test failure because the remainder of the tests expect it do throw some exception, but doesn't check what exception :)

I may have a deeper look at this later because there's a possibility that those tests are passing accidentally because the resulting exceptions aren't being checked very deeply.

@skaji
Copy link
Collaborator

skaji commented Aug 29, 2017

I confirmed that t/Crust-Middleware/lint.t failed; we can reduce that failure to a simple case:

❯ perl6 -v
This is Rakudo version 2017.08-30-g94fe65dbe built on MoarVM version 2017.08.1-19-g151a25634
implementing Perl 6.c.
❯ perl6 -e 'my $a = [ Content-Length => 10 ]; $a.pairup'
P6opaque: no such attribute '$!value' in type Pair when trying to get a value
  in block <unit> at -e line 1

According to the spec of P6W and an implementation of P6W server,
HTTP response headers must be Lists of Paris.

So I think we should check that by something like

diff --git lib/Crust/Middleware/Lint.pm6 lib/Crust/Middleware/Lint.pm6
index 892e7a9..0c490b7 100644
--- lib/Crust/Middleware/Lint.pm6
+++ lib/Crust/Middleware/Lint.pm6
@@ -63,13 +63,8 @@ my sub validate-ret(@ret) {
 
     my $copy = @ret[1];
 
-    {
-        $copy.pairup();
-        CATCH {
-            default {
-                die 'The number of response headers needs to be even, not odd(', $copy, ')';
-            }
-        }
+    unless all(@($copy)) ~~ Pair {
+        die 'Response headers must be a List of Pairs, but ', $copy;
     }
 
     for $copy.kv -> $i, $v {
diff --git t/Crust-Middleware/lint.t t/Crust-Middleware/lint.t
index d4ffd2b..8fbd17e 100644
--- t/Crust-Middleware/lint.t
+++ t/Crust-Middleware/lint.t
@@ -146,7 +146,7 @@ subtest {
             sub (%env) { start { 200, ['invalid'], ['hello'] } }
         );
         dies-ok({await $code(%env)});
-    }, 'Should die because response header has odd elements';
+    }, 'Should die because response header is not a List of Pairs';
 
     subtest {
         my $code = Crust::Middleware::Lint.new(

@skaji
Copy link
Collaborator

skaji commented Aug 30, 2017

The spec also says that

The headers MUST be a List of Pairs or an object that when coerced into a List becomes a List of Pairs.

What does "coerced" mean here? :)

@jonathanstowe
Copy link
Contributor Author

I guess you could have a type that implements a .list method which returns a list of pars.

The previous implementation would allow an even sized list of key, value items to be acceptable. So <a 1 b 2> would be acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants