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

Plugins and non-zero number_of_redirections #11676

Open
ywkaras opened this issue Aug 13, 2024 · 3 comments
Open

Plugins and non-zero number_of_redirections #11676

ywkaras opened this issue Aug 13, 2024 · 3 comments

Comments

@ywkaras
Copy link
Contributor

ywkaras commented Aug 13, 2024

If a plugin (header_rewrite for example) sets the reply status to 301, and proxy.config.http.number_of_redirections is non-zero, will it follow the redirect (and should it)?

@JosiahWI
Copy link
Contributor

Yes, it will follow it. Here's a quick way to reproduce.

AuTest Setup

Apply the following patch to the header_rewrite_url_glob AuTest:

diff --git a/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_url_glob.test.py b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_url_glob.test.py
index 006acaabb..02816f033 100644
--- a/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_url_glob.test.py
+++ b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_url_glob.test.py
@@ -35,9 +35,10 @@ server.addResponse("sessionfile.log", request_header, response_header)
 ts.Disk.records_config.update(
     {
         'proxy.config.url_remap.remap_required': 0,
+        'proxy.config.http.number_of_redirections': 1,
         'proxy.config.diags.debug.enabled': 1,
         'proxy.config.diags.show_location': 0,
-        'proxy.config.diags.debug.tags': 'header',
+        'proxy.config.diags.debug.tags': 'header|http|redirect',
     })
 ts.Setup.CopyAs('rules/glob_set_redirect.conf', Test.RunDirectory)
 

And make the following modification to the debug tags:

Log Observation

The AuTest passes and it's not obvious that a redirect happened without examining traffic.out.

The first relevant part of traffic.out is the initial response handling, where the redirect gets initiated. Note redirect_required: 1:

+++++++++ Base Header for Building Response +++++++++
-- State Machine Id: 0
HTTP/1.1 301 Moved Permanently
Connection: close
Location: http://redirect.com/here
Date: Wed, 14 Aug 2024 15:10:48 GMT

+++++++++ Proxy's Response 2 +++++++++
-- State Machine Id: 0
HTTP/1.1 301 Moved Permanently
Location: http://redirect.com/here
Date: Wed, 14 Aug 2024 15:10:48 GMT
Age: 0
Connection: keep-alive
Server: ATS/10.1.0

[Aug 14 15:10:48.274] [ET_NET 0] DIAG: (http) [0] State Transition: SM_ACTION_ORIGIN_SERVER_OPEN -> SM_ACTION_SERVER_READ
[Aug 14 15:10:48.274] [ET_NET 0] DIAG: (http_redirect) [0] redirect_required: 1
[Aug 14 15:10:48.274] [ET_NET 0] DIAG: (http_redirect) [0] enable_redirection 1
[Aug 14 15:10:48.274] [ET_NET 0] DIAG: (http_redirect) [0] redirect_required: 1
[Aug 14 15:10:48.274] [ET_NET 0] DIAG: (http_redirect) [0] redirect url: http://redirect.com/here

The second relevant part of traffic.out is the DNS Lookup for the URI in the Location: header. This is where it ATS decides to actually follow through with the redirect.

[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_trans) [0] Entering HttpTransact::OSDNSLookup
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_seq) [0] DNS Lookup successful
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_trans) [0] DNS lookup for O.S. successful IP: 54.208.19.128:80
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_trans) [0] [OSDNSLookup] Mapped action - 2 for family 2.
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_trans) [0] Invalid redirect address. Following
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_trans) Next action SM_ACTION_API_OS_DNS; HandleCacheOpenReadMiss
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http) [0] State Transition: SM_ACTION_DNS_LOOKUP -> SM_ACTION_API_OS_DNS
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_trans) [0] --- MISS
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_seq) [0] Miss in cache
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_trans) [0] client_ip_set = 0
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_trans) [0] inserted request header 'Client-ip: 127.0.0.1'
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_trans) [0] Appended connecting client's (127.0.0.1) to the X-Forwards header
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_trans) [0] removing host name from url
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_trans) [0] request not like cacheable and conditional headers not removed
[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http_trans) [0] request_sent_time: 1723648921
+++++++++ Proxy's Request +++++++++
-- State Machine Id: 0
HEAD /here HTTP/1.1
Host: redirect.com
User-Agent: curl/8.8.0
Accept: */*
Client-ip: 127.0.0.1
X-Forwarded-For: 127.0.0.1
Via: http/1.1 traffic_server[f9e9af18-f6cc-41c4-a37e-b09adb1bdaee] (ApacheTrafficServer/10.1.0)

[Aug 14 15:22:01.038] [ET_NET 0] DIAG: (http) [0] State Transition: SM_ACTION_API_OS_DNS -> SM_ACTION_ORIGIN_SERVER_OPEN

This gets another 301 response and sends it back to the client because the maximum number of redirects (1) has been reached. The response is what the AuTest expects, so it passes.

@ywkaras
Copy link
Contributor Author

ywkaras commented Aug 14, 2024

When I added that test I rashly assumed that redirect.com was an invalid domain name.

@ywkaras
Copy link
Contributor Author

ywkaras commented Aug 14, 2024

Looks like that's one of the tests that needs to make a dummy DNS server with the Au test extension.

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

No branches or pull requests

3 participants