You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bryancall (via GitHub)" <gi...@apache.org> on 2025/05/16 17:48:59 UTC
[PR] Updated testing and fixed bug in SSLSNIConfig [trafficserver]
bryancall opened a new pull request, #12248:
URL: https://github.com/apache/trafficserver/pull/12248
If the `sni.yaml` configuration file contained multiple wildcard entries, an incorrect wildcard entry could be matched instead of the exact match listed first in the file.
This issue occurred because the `rank` for wildcard entries was set to 0 (the default value) and was not updated in the move constructor when the list was resized to accommodate a new entry.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Updated testing and fixed bug in SSLSNIConfig [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #12248:
URL: https://github.com/apache/trafficserver/pull/12248#issuecomment-2887686767
Hey @masaori335, you added in the performance enhancements and ranking to begin with, so you should know more about this code than anyone else. Can you please review the bug fix and the added tests?
If you can think of more tests I can add them too. I have another PR to change the PCRE regexes to use the `Regex` class (PCRE2 wrapper) and I want to make sure the tests will catch any new bugs.
I have to check to see if we are covering case were we verify the capture group.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Updated testing and fixed bug in SSLSNIConfig [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #12248:
URL: https://github.com/apache/trafficserver/pull/12248#discussion_r2100626590
##########
src/iocore/net/SSLSNIConfig.cc:
##########
@@ -77,6 +77,7 @@ NamedElement::operator=(NamedElement &&other)
if (this != &other) {
match = std::move(other.match);
inbound_port_ranges = std::move(other.inbound_port_ranges);
+ rank = other.rank;
Review Comment:
This makes sense
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Updated testing and fixed bug in SSLSNIConfig [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #12248:
URL: https://github.com/apache/trafficserver/pull/12248#issuecomment-2887463883
Looking at the updated docs globs in the middle are no longer supported. It adds the entry with the asterisk in the middle to the exact match list with no warning.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Updated testing and fixed bug in SSLSNIConfig [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #12248:
URL: https://github.com/apache/trafficserver/pull/12248#issuecomment-2912535010
Cherry-picked to 10.1.x branch
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Updated testing and fixed bug in SSLSNIConfig [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on code in PR #12248:
URL: https://github.com/apache/trafficserver/pull/12248#discussion_r2093675510
##########
src/iocore/net/SSLSNIConfig.cc:
##########
@@ -77,6 +77,7 @@ NamedElement::operator=(NamedElement &&other)
if (this != &other) {
match = std::move(other.match);
inbound_port_ranges = std::move(other.inbound_port_ranges);
+ rank = other.rank;
Review Comment:
This is the bug fix. ⬆️
The rest of the changes are more tests and an updated config.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Updated testing and fixed bug in SSLSNIConfig [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall merged PR #12248:
URL: https://github.com/apache/trafficserver/pull/12248
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Updated testing and fixed bug in SSLSNIConfig [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #12248:
URL: https://github.com/apache/trafficserver/pull/12248#discussion_r2100641904
##########
src/iocore/net/unit_tests/sni_conf_test.yaml:
##########
@@ -36,3 +36,16 @@ sni:
http2_buffer_water_mark: 256
- fqdn: foo.bar.com
http2: false
+
+# test with mixed-case
+- fqdn: "MiXeDcAsE.foo.com"
+ http2: true
+ http2_buffer_water_mark: 256
+ inbound_port_ranges: 31337
+
+# test with mixed-case glob
+- fqdn: "*.MiXeDcAsE.com"
+ http2: false
+
+# test glob in the middle, this will be an exact match
+- fqdn: "cat.*.com"
Review Comment:
Discussion around the #9736, we concluded don't allow this. This should be error.
> 1. Allow single left-most `*` in the fqdn field
https://docs.trafficserver.apache.org/en/latest/admin-guide/files/sni.yaml.en.html
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Updated testing and fixed bug in SSLSNIConfig [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #12248:
URL: https://github.com/apache/trafficserver/pull/12248#issuecomment-2898466572
The main change seems reasonable, but the wildcard in middle is we don't support. If you rush, we can merge this and deal with the issue later.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Updated testing and fixed bug in SSLSNIConfig [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on code in PR #12248:
URL: https://github.com/apache/trafficserver/pull/12248#discussion_r2093675510
##########
src/iocore/net/SSLSNIConfig.cc:
##########
@@ -77,6 +77,7 @@ NamedElement::operator=(NamedElement &&other)
if (this != &other) {
match = std::move(other.match);
inbound_port_ranges = std::move(other.inbound_port_ranges);
+ rank = other.rank;
Review Comment:
This is the bug fix. ⬆️
The rest is just more tests and an updated config.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Updated testing and fixed bug in SSLSNIConfig [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #12248:
URL: https://github.com/apache/trafficserver/pull/12248#issuecomment-2898439099
@masaori335 Can you take a look at this? I would like to get it merged in, I have another PR that relies on this one.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Updated testing and fixed bug in SSLSNIConfig [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #12248:
URL: https://github.com/apache/trafficserver/pull/12248#discussion_r2102732795
##########
src/iocore/net/unit_tests/sni_conf_test.yaml:
##########
@@ -36,3 +36,16 @@ sni:
http2_buffer_water_mark: 256
- fqdn: foo.bar.com
http2: false
+
+# test with mixed-case
+- fqdn: "MiXeDcAsE.foo.com"
+ http2: true
+ http2_buffer_water_mark: 256
+ inbound_port_ranges: 31337
+
+# test with mixed-case glob
+- fqdn: "*.MiXeDcAsE.com"
+ http2: false
+
+# test glob in the middle, this will be an exact match
+- fqdn: "cat.*.com"
Review Comment:
This is for these tests for invalid?
https://github.com/apache/trafficserver/pull/12248/files#diff-40f6583cfe88d8e4ea494c95bf3dbceed8a172c40836afe607f9ff7c1981e3a8R160-R171
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org