Skip to content
This repository has been archived by the owner on May 24, 2022. It is now read-only.

Handle prefix removal when entire path is removed #46

Merged
2 commits merged into from
Jun 16, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jun 5, 2020

The req_path action allows for removing the entire matched path,
which could lead to an empty "path" variable in Varnish. This
did not render the config invalid but caused a 503 to be thrown
when the rule was triggered. The config template now includes a
check for an empty path variable and handles the case correctly.

Resolves #37

The req_path action allows for removing the entire matched path,
which could lead to an empty "path" variable in Varnish. This
did not render the config invalid but caused a 503 to be thrown
when the rule was triggered. The config template now includes a
check for an empty path variable and handles the case correctly.
@ghost ghost requested a review from splushii June 5, 2020 09:08
@ghost ghost linked an issue Jun 5, 2020 that may be closed by this pull request
Copy link

@splushii splushii left a comment

Choose a reason for hiding this comment

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

LGTM! 👍 Just the one minor thing

Sorry about the late review! The notification drowned and I didn't get any email :S

I also noticed when doing make release-test locally that it seems broken for both python 3.8 as well as pip 20

orm/templates/varnish.vcl.j2 Outdated Show resolved Hide resolved
@splushii
Copy link

Oh, regarding the pip 20 issue. Saw now that pip 19.0.3 is explicitly installed on the v1 branch. So it's only broken on the master branch.

@ghost
Copy link
Author

ghost commented Jun 15, 2020

Oh, regarding the pip 20 issue. Saw now that pip 19.0.3 is explicitly installed on the v1 branch. So it's only broken on the master branch.

I say we make a new issue for this.

@ghost ghost requested a review from splushii June 15, 2020 07:03
@ghost ghost merged commit edfa577 into v1 Jun 16, 2020
@ghost ghost deleted the I37-empty-path-string-after-rewrite branch June 16, 2020 14:03
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

503 returned for requests to exact paths matched with "begins_with"
2 participants