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: prevent infinite loop in enumerate_wildcard_locations #1064

Merged
merged 1 commit into from
Dec 10, 2023

Conversation

egormkn
Copy link
Contributor

@egormkn egormkn commented Nov 23, 2023

This pull request fixes two issues in enumerate_wildcard_locations function:

  1. When a wildcard domain passed to the function, it never returns. Even though wildcard domains are not supported at the moment, I think, infinite loop should be fixed. Currently any wildcard domain in any container configuration completely hangs the service.
  2. In descending_wildcard_locations the first occurrence of last_label was removed, which might generate wrong wildcards:
#!/bin/bash

source ./functions.sh
enumerate_wildcard_locations test.com.example.com

# Output:
# *.com.example.com
# *.example.com
# test.example.com.*  <-- should be test.com.example.*
# test.example.*      <-- should be test.com.*
# test.*

@buchdag
Copy link
Member

buchdag commented Dec 8, 2023

@egormkn thanks for this fix 👍

Do you think it would be possible to update the tests so that the bug you're fixing is covered ? I don't have permission to push to your fork.

@buchdag
Copy link
Member

buchdag commented Dec 8, 2023

--- a/test/tests/location_config/expected-std-out.txt
+++ b/test/tests/location_config/expected-std-out.txt
@@ -1,7 +1,9 @@
-*.bar.baz.example.com
-*.baz.example.com
+*.bar.baz.com.example.com
+*.baz.com.example.com
+*.com.example.com
 *.example.com
-foo.bar.baz.example.*
+foo.bar.baz.com.example.*
+foo.bar.baz.com.*
 foo.bar.baz.*
 foo.bar.*
 foo.*
--- a/test/tests/location_config/run.sh
+++ b/test/tests/location_config/run.sh
@@ -54,7 +54,7 @@ function check_location {
}

# check the wildcard location enumeration function
-docker exec "$le_container_name" bash -c 'source /app/functions.sh; enumerate_wildcard_locations foo.bar.baz.example.com'
+docker exec "$le_container_name" bash -c 'source /app/functions.sh; enumerate_wildcard_locations foo.bar.baz.com.example.com'

# default configuration file should be empty
config_path="$vhost_path/default"

@buchdag buchdag added the type/fix PR for a bug fix label Dec 8, 2023
@buchdag buchdag merged commit 97b25e7 into nginx-proxy:main Dec 10, 2023
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/fix PR for a bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants