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

Fix header forwarding test #1263

Merged
merged 6 commits into from
Jan 16, 2022
Merged

Conversation

Gabriella439
Copy link
Contributor

Partial fix for #1260

The nginx configuration for test.dhall-lang.org wasn't correctly
checking the headers because the fully materialized configuration
looked like this:

server {
        …
        server_name test.dhall-lang.org ;
        …
        location / {
                proxy_pass https://raw.githubusercontent.com;
                …
                rewrite ^/?$ https://store.dhall-lang.org/Prelude-v21.1.0 redire
ct;
                rewrite ^/(v[^/]+)$ https://github.com/dhall-lang/dhall-lang/tre
e/$1/Prelude redirect;
                rewrite ^/(v[^/]+)/(.*)$ /dhall-lang/dhall-lang/$1/Prelude/$2 br
eak;
                rewrite ^/(.*)$ /dhall-lang/dhall-lang/v21.1.0/Prelude/$1 break;
                if ($http_test = "") {
                        return 403;
                }
        }

… and the rewrite rules were taking precedence over the http_test check.

This change fixes that by redoing the test such that only
test.dhall-lang.org/{foo,bar} require the Test header and those two
imports are used for the test instead. A side benefit of this change is
that the test is now not as brittle (since it won't break if we
change Prelude/Bool/package.dhall)

Partial fix for #1260

The `nginx` configuration for `test.dhall-lang.org` wasn't correctly
checking the headers because the fully materialized configuration
looked like this:

```
server {
        …
        server_name test.dhall-lang.org ;
        …
        location / {
                proxy_pass https://raw.githubusercontent.com;
                …
                rewrite ^/?$ https://store.dhall-lang.org/Prelude-v21.1.0 redire
ct;
                rewrite ^/(v[^/]+)$ https://github.com/dhall-lang/dhall-lang/tre
e/$1/Prelude redirect;
                rewrite ^/(v[^/]+)/(.*)$ /dhall-lang/dhall-lang/$1/Prelude/$2 br
eak;
                rewrite ^/(.*)$ /dhall-lang/dhall-lang/v21.1.0/Prelude/$1 break;
                if ($http_test = "") {
                        return 403;
                }
        }
```

… and the `rewrite` rules were taking precedence over the `http_test` check.

This change fixes that by redoing the test such that only
`test.dhall-lang.org/{foo,bar}` require the `Test` header and those two
imports are used for the test instead.  A side benefit of this change is
that the test is now not as brittle (since it won't break if we
change `Prelude/Bool/package.dhall`)
Gabriella439 and others added 2 commits January 8, 2022 17:46
… as caught by @hagl

Co-authored-by: Harald Gliebe <harald.gliebe@gmail.com>
Copy link
Contributor

@hagl hagl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one outdated comment, otherwise looks good to me

Gabriella439 and others added 2 commits January 10, 2022 08:17
… as suggested by @hagl

Co-authored-by: Harald Gliebe <harald.gliebe@gmail.com>
@Gabriella439 Gabriella439 merged commit 8ca9eda into master Jan 16, 2022
@Gabriella439 Gabriella439 deleted the gabriella/fix_header_forwarding_test branch January 16, 2022 22:20
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