Skip to content

Fix pattern matches#2657

Merged
drwetter merged 1 commit intotestssl:3.2from
dcooper16:fix_pattern_match
Feb 15, 2025
Merged

Fix pattern matches#2657
drwetter merged 1 commit intotestssl:3.2from
dcooper16:fix_pattern_match

Conversation

@dcooper16
Copy link
Copy Markdown
Collaborator

Describe your changes

This PR fixes three lines of code that use Bash substring matching. In each case, a list of strings to match was enclosed in brackets. This resulted in a match if the string to test contained any character from any of the strings to match. This PR fixes the issue by removing the brackets.

I have tested the change in run_npn(), but have not tested the changes in the other two functions.

(The bugs were introduced in b8e9b09 and 8149c2d)

What is your pull request about?

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Typo fix
  • Documentation update
  • Update of other files

If it's a code change please check the boxes which are applicable

  • For the main program: My edits contain no tabs and the indentation is five spaces
  • I've read CONTRIBUTING.md and Coding_Convention.md
  • I have tested this fix against >=2 hosts and I couldn't spot a problem
  • I have tested this new feature against >=2 hosts which show this feature and >=2 host which does not (in order to avoid side effects) . I couldn't spot a problem
  • For the new feature I have made corresponding changes to the documentation and / or to help()
  • If it's a bigger change: I added myself to CREDITS.md (alphabetical order) and the change to CHANGELOG.md

This commit fixes three lines of code that use Bash substring matching. In each case, a list of strings to match was enclosed in brackets. This resulted in a match if the string to test contained any character from any of the strings to match. This commit fixes the issue by removing the brackets.

(The bugs were introduced in testssl@b8e9b09 and testssl@8149c2d)
@drwetter drwetter merged commit 2baaf61 into testssl:3.2 Feb 15, 2025
@drwetter
Copy link
Copy Markdown
Collaborator

Good catch!

@dcooper16 dcooper16 deleted the fix_pattern_match branch February 18, 2025 13:47
@drwetter
Copy link
Copy Markdown
Collaborator

Forgot to ask, @dcooper16 : how did you spot that?

@dcooper16
Copy link
Copy Markdown
Collaborator Author

Forgot to ask, @dcooper16 : how did you spot that?

I noticed it when I was testing #2659. I didn't understand the comment (now comes a strange thing: "Protocols advertised by server:" is empty but connection succeeded), so I tried to trigger the "server response was ambiguous" output using something like$OPENSSL s_server -nextprotoneg notaprotocol,h3. When the error message wasn't displaying as expected, I eventually figured out that it was the brackets in the conditional that were the problem.

I then did a search for =~ [. There were 8 matches, 5 that looked okay and 3 where it seemed clear the brackets did not belong.

drwetter added a commit that referenced this pull request May 21, 2025
The rhs of the pattern was off by one byte and it worked in practise until recent PR #2657.

This fixes #2691 for 3.2 .
drwetter added a commit that referenced this pull request May 21, 2025
The rhs of the pattern was off by one byte and it worked in practise until recent PR #2657.

This fixes #2691 for 3.2 .
@drwetter drwetter mentioned this pull request May 21, 2025
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants