You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2020/10/08 19:48:13 UTC
[GitHub] [trafficserver] ywkaras opened a new pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
ywkaras opened a new pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262
The original TSUrlSchemeGet() is renamed to TSUrlRawSchemeGet().
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#issuecomment-761918735
Now I don't see a test for absolute form request URL.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r508914961
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
I prefer consistently using curl for all HTTP versions as long as curl can do the same thing, because I can clearly see the tests do the same things on different versions.
If you think we all should use printf and nc for H1 test cases, you should make another PR to make the change for all H1 tests, and let's see what other people think. I don't think this change has to be done on this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r509873556
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
https://tools.ietf.org/html/rfc7230#section-5.3.2
> When making a request to a proxy, other than a CONNECT or server-wide
OPTIONS request (as detailed below), a client MUST send the target
URI in absolute-form as the request-target.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r509350047
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
How do I control whether or not curl puts the scheme and host in the HTTP Request URL?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r504188900
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
To me it's more clear what the exact contents of the HTTP message is when I use nc rather than curl.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r501979013
##########
File path: tests/gold_tests/pluginTest/tsapi/log.gold
##########
@@ -1,14 +1,87 @@
Global: event=TS_EVENT_HTTP_TXN_START
Global: event=TS_EVENT_HTTP_READ_REQUEST_HDR
TSHttpTxnEffectiveUrlStringGet(): http://mYhOsT.teSt:SERVER_PORT/
+Client Request:
TSHttpHdrEffectiveUrlBufGet(): http://myhost.test:SERVER_PORT/
+TSUrlSchemeGet(): http
+TSUrlRawSchemeGet(): http
+TSUrlPortGet(): 80
Transaction: event=TS_EVENT_HTTP_READ_REQUEST_HDR
TSHttpTxnEffectiveUrlStringGet(): http://mYhOsT.teSt:SERVER_PORT/
+Client Request:
TSHttpHdrEffectiveUrlBufGet(): http://myhost.test:SERVER_PORT/
+TSUrlSchemeGet(): http
+TSUrlRawSchemeGet(): http
+TSUrlPortGet(): 80
+Global: event=TS_EVENT_HTTP_SEND_REQUEST_HDR
+TSHttpTxnEffectiveUrlStringGet(): http://127.0.0.1:SERVER_PORT/
+Request To Server:
+TSHttpHdrEffectiveUrlBufGet(): 127.0.0.1:SERVER_PORT/
Review comment:
This should be fixed in a future PR. Should include the implicit scheme.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r501978141
##########
File path: tests/tools/plugins/test_tsapi.cc
##########
@@ -28,14 +28,6 @@ Regression testing code for TS API. Not comprehensive, hopefully will be built
#include <ts/ts.h>
#include <tscpp/util/PostScript.h>
-// TSReleaseAssert() doesn't seem to produce any logging output for a debug build, so do both kinds of assert.
-//
-#define ALWAYS_ASSERT(EXPR) \
- { \
- TSAssert(EXPR); \
- TSReleaseAssert(EXPR); \
- }
-
Review comment:
Some unrelated cleanup, included in this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r508937535
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
@bryancall what is your thinking about this? @d2r ?
I have one test run with the scheme and host in the URL, and one with just the path in the URL (with the host in a host header and no explicit scheme). Have you seen how long the manpage for curl is? Does it even say whether or not the scheme/host are in the request URL? If I have to revisit this test case at some point in the future, I'd rather not have to have to dig into all the quirks of curl to understand it, given that there is a more straight-forward alternative.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#issuecomment-731256816
@SolidWallOfCode @shinrich what is your opinions as to whether we should prohibit the use of netcat in new Au tests? I think curl is not generally the best option for testing, as it doesn't allow one to exactly and explicitly (thus readably) specify the contents of the HTTP request. netcat and openssl s_client allow you to do this (in conjunction with bash printf/echo -e). The only case where there is no alternative to curl seems to be for H2.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r509824501
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
`curl -x http://yourats/ http://foo/` puts them in RequestURL.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r501979854
##########
File path: tests/gold_tests/pluginTest/tsapi/log.gold
##########
@@ -1,14 +1,87 @@
Global: event=TS_EVENT_HTTP_TXN_START
Global: event=TS_EVENT_HTTP_READ_REQUEST_HDR
TSHttpTxnEffectiveUrlStringGet(): http://mYhOsT.teSt:SERVER_PORT/
+Client Request:
TSHttpHdrEffectiveUrlBufGet(): http://myhost.test:SERVER_PORT/
+TSUrlSchemeGet(): http
+TSUrlRawSchemeGet(): http
+TSUrlPortGet(): 80
Review comment:
This should return SERVER_PORT, to be fixed in future PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r510989918
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
If you use --proxy, curl thinks the server is a forward proxy regardless of your server configuration, and it makes the RequestURL you need.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r509829801
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
What do you mean? -x is the abbreviation of --proxy. What does that have to do with controlling whether or not the scheme and host are in the URL in the HTTP request?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r508947324
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
I'd say "Have you seen how long the H2 spec is?". I would definitely not make a binary data and send it with "openssl s_client" for this test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r512118597
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
OK I think I see what you mean now:
```
$ curl --verbose http://httpbin.org/ip 2>&1 | grep '^> '
> GET /ip HTTP/1.1
> Host: httpbin.org
> User-Agent: curl/7.66.0-DEV
> Accept: */*
>
$ curl --verbose -x httpbin.org:80 http://httpbin.org/ip 2>&1 | grep '^> '
> GET http://httpbin.org/ip HTTP/1.1
> Host: httpbin.org
> User-Agent: curl/7.66.0-DEV
> Accept: */*
> Proxy-Connection: Keep-Alive
>
$
```
But to me this makes the test harder rather than easier to understand and maintain. Notice that curl inserts several headers that are not relevant to the test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r501979854
##########
File path: tests/gold_tests/pluginTest/tsapi/log.gold
##########
@@ -1,14 +1,87 @@
Global: event=TS_EVENT_HTTP_TXN_START
Global: event=TS_EVENT_HTTP_READ_REQUEST_HDR
TSHttpTxnEffectiveUrlStringGet(): http://mYhOsT.teSt:SERVER_PORT/
+Client Request:
TSHttpHdrEffectiveUrlBufGet(): http://myhost.test:SERVER_PORT/
+TSUrlSchemeGet(): http
+TSUrlRawSchemeGet(): http
+TSUrlPortGet(): 80
Review comment:
This should return SERVER_PORT, to be fixed in future PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#issuecomment-716920814
Hopefully we can discuss tools for generating test requests during the summit. We had a long discussion about this in our (Vz) weekly meeting.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r510180460
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
I believe that, under the terminology of the RFC, the term "proxy" only applies to forward proxies. A reverse proxy is called a "gateway". I don't think the restriction you reference applies to gateways.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r501978141
##########
File path: tests/tools/plugins/test_tsapi.cc
##########
@@ -28,14 +28,6 @@ Regression testing code for TS API. Not comprehensive, hopefully will be built
#include <ts/ts.h>
#include <tscpp/util/PostScript.h>
-// TSReleaseAssert() doesn't seem to produce any logging output for a debug build, so do both kinds of assert.
-//
-#define ALWAYS_ASSERT(EXPR) \
- { \
- TSAssert(EXPR); \
- TSReleaseAssert(EXPR); \
- }
-
Review comment:
Some unrelated cleanup, included in this PR.
##########
File path: tests/gold_tests/pluginTest/tsapi/log.gold
##########
@@ -1,14 +1,87 @@
Global: event=TS_EVENT_HTTP_TXN_START
Global: event=TS_EVENT_HTTP_READ_REQUEST_HDR
TSHttpTxnEffectiveUrlStringGet(): http://mYhOsT.teSt:SERVER_PORT/
+Client Request:
TSHttpHdrEffectiveUrlBufGet(): http://myhost.test:SERVER_PORT/
+TSUrlSchemeGet(): http
+TSUrlRawSchemeGet(): http
+TSUrlPortGet(): 80
Transaction: event=TS_EVENT_HTTP_READ_REQUEST_HDR
TSHttpTxnEffectiveUrlStringGet(): http://mYhOsT.teSt:SERVER_PORT/
+Client Request:
TSHttpHdrEffectiveUrlBufGet(): http://myhost.test:SERVER_PORT/
+TSUrlSchemeGet(): http
+TSUrlRawSchemeGet(): http
+TSUrlPortGet(): 80
+Global: event=TS_EVENT_HTTP_SEND_REQUEST_HDR
+TSHttpTxnEffectiveUrlStringGet(): http://127.0.0.1:SERVER_PORT/
+Request To Server:
+TSHttpHdrEffectiveUrlBufGet(): 127.0.0.1:SERVER_PORT/
Review comment:
This should be fixed in a future PR. Should include the implicit scheme.
##########
File path: tests/gold_tests/pluginTest/tsapi/log.gold
##########
@@ -1,14 +1,87 @@
Global: event=TS_EVENT_HTTP_TXN_START
Global: event=TS_EVENT_HTTP_READ_REQUEST_HDR
TSHttpTxnEffectiveUrlStringGet(): http://mYhOsT.teSt:SERVER_PORT/
+Client Request:
TSHttpHdrEffectiveUrlBufGet(): http://myhost.test:SERVER_PORT/
+TSUrlSchemeGet(): http
+TSUrlRawSchemeGet(): http
+TSUrlPortGet(): 80
Review comment:
This should return SERVER_PORT, to be fixed in future PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r512362186
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
> Notice that curl inserts several headers that are not relevant to the test.
I think they are relevant because they may change ATS's behavior. I guess major clients send Proxy-Client when they use absolute form, so testing the behavior without the header may be missing the point. And we can easily make natural requests by using curl. I don't think I can always do it manually.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r560390309
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -47,45 +52,50 @@
'proxy.config.proxy_name': 'Poxy_Proxy', # This will be the server name.
'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir),
- 'proxy.config.url_remap.remap_required': 0,
- 'proxy.config.diags.debug.enabled': 0,
+ 'proxy.config.url_remap.remap_required': 1,
+ 'proxy.config.diags.debug.enabled': 1,
'proxy.config.diags.debug.tags': 'http|test_tsapi',
})
ts.Disk.ssl_multicert_config.AddLine(
'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key'
)
+rp = os.path.join(Test.TestDirectory, '.libs', 'test_tsapi.so')
+
ts.Disk.remap_config.AddLine(
- "map http://myhost.test:{0} http://127.0.0.1:{0}".format(server.Variables.Port)
+ "map http://myhost.test http://127.0.0.1:{0} @plugin={1} @plugin={1}".format(server.Variables.Port, rp)
)
ts.Disk.remap_config.AddLine(
- "map https://myhost.test:{0} http://127.0.0.1:{0}".format(server.Variables.Port)
+ "map https://myhost.test:123 http://127.0.0.1:{0} @plugin={1} @plugin={1}".format(server.Variables.Port, rp)
)
-Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'test_tsapi.so'), ts)
-
tr = Test.AddTestRun()
# Probe server port to check if ready.
tr.Processes.Default.StartBefore(server, ready=When.PortOpen(server.Variables.Port))
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt" hTtP://loCalhOst:{}/'.format(ts.Variables.port)
+)
+tr.Processes.Default.ReturnCode = 0
+
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = (
+ 'curl --verbose --ipv4 --proxy localhost:{} http://mYhOsT.teSt/xYz'.format(ts.Variables.port)
Review comment:
OK, this generates an absolute URL.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] randall commented on a change in pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
randall commented on a change in pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#discussion_r504105054
##########
File path: tests/gold_tests/pluginTest/tsapi/tsapi.test.py
##########
@@ -71,7 +71,15 @@
tr.Processes.Default.StartBefore(Test.Processes.ts)
#
tr.Processes.Default.Command = (
- 'curl --verbose --ipv4 --header "Host: mYhOsT.teSt:{0}" hTtP://loCalhOst:{1}/'.format(server.Variables.Port, ts.Variables.port)
+ r'printf "GET / HTTP/1.1\r\nHost: mYhOsT.teSt:{0}\r\n\r\n" | nc localhost {1}'.format(
Review comment:
Why nc?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras merged pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras merged pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #7262: Make TSUrlSchemeGet() return scheme implied by URL type when there is no explicit scheme.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #7262:
URL: https://github.com/apache/trafficserver/pull/7262#issuecomment-761233476
OK @maskit no more netcat.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org