feature: added --- sni Support TLS http2 and SNI #68
feature: added --- sni Support TLS http2 and SNI #68vislee wants to merge 3 commits intoopenresty:masterfrom
Conversation
lib/Test/Nginx/Socket.pm
Outdated
|
|
||
| if (use_http2($block)) { | ||
| my $isHttp2 = use_http2($block); | ||
| if ($isHttp2 && $isHttp2 == 1) { |
There was a problem hiding this comment.
We do not use "camel case" identifier names in this Perl project. We use "snake case" consistently.
lib/Test/Nginx/Util.pm
Outdated
|
|
||
| if (defined $block->sni) { | ||
| $block->set_value("test_nginx_enabled_http2_sni", 1); | ||
| return 2; |
There was a problem hiding this comment.
The use_http2 function returns a boolean value. It's not a good idea to make it return more complex encodings.
lib/Test/Nginx/Socket.pm
Outdated
| } | ||
|
|
||
| if (use_http2($block) == 2) { | ||
| $link = "https://$ServerName:$ServerPortForClient$uri"; |
There was a problem hiding this comment.
SSL support need proper server certificate and private keys. And it's a burden for the test writer to prepare such things themselves. We should make HTTP/2 over TLS test mode work on most of the existing test suite written for HTTP/1. So we should automatically generate such self-signed server certificate/key pair ourselves in the test scaffold.
lib/Test/Nginx/Socket.pm
Outdated
|
|
||
| Enable SNI mode for an individual test block by specifying the L<sni> section, as in | ||
|
|
||
| --- sni |
There was a problem hiding this comment.
I'm afraid --- tls is a better name than --- sni since we are not specifying a concrete SNI name here anyway.
There was a problem hiding this comment.
Thanks for your review and comments. I'll change it.
| =head2 http2 | ||
|
|
||
| Enforces the test scaffold to use the HTTP/2 wire protocol to send the test request. | ||
| Also, you can set tls section using the HTTP/2 over TLS protocol and SNI(Server Name Indication). |
There was a problem hiding this comment.
Do we really need to mention SNI here? It can be implicit I think. The user cannot specify their own SNI names now anyway.
| $link = "http://$server:$port$uri"; | ||
| if (use_http2($block) && defined $block->tls) { | ||
| my $server_name = $ServerName; | ||
| $link = "https://$server_name:$port$uri"; |
There was a problem hiding this comment.
As I mentioned in my previous round of review, the test scaffold should automatically generate self-signed server certificate and key files and automatically make use of them in the automatically generated nginx.conf. All such details should be transparent. Also, we can introduce a TEST_NGINX_USE_HTTP2_TLS=1 system environment to enable this for any existing tests without any edits in the tests themselves?
There was a problem hiding this comment.
@agentzh I think it can be split into two parts. Listening on HTTPS/2 and making HTTPS/2 requests from Test::Nginx.
I have tests that are using custom certificates and want to keep it that way, but would like to make HTTPS/2 request from Test::Nginx to verify them. IMO those are two different problems.
I see the workaround in ssl_certificate_by_lua tests (using cosockets), but having first class support in Test::Nginx would be nice.
3 participants
No description provided.