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

KAFKA-15500: Correct SslPrincipalMapper.java #14441

Merged
merged 1 commit into from
Sep 29, 2023
Merged

Conversation

rykovsi
Copy link
Contributor

@rykovsi rykovsi commented Sep 25, 2023

Correct this "&" to "&&"

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Correct this "&" to "&&"
@rykovsi rykovsi changed the title Correct SslPrincipalMapper.java [KAFKA-15500] Correct SslPrincipalMapper.java Sep 25, 2023
Copy link
Member

@mimaison mimaison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Thanks for the PR

@divijvaidya
Copy link
Contributor

Thank you submitting this fix @rykovsi. I was wondering if we could add a unit test in SslPrincipalMapperTest that would fail with the bug and pass after this is fixed. It would help us detect such bugs in future.

@divijvaidya divijvaidya changed the title [KAFKA-15500] Correct SslPrincipalMapper.java KAFKA-15500: Correct SslPrincipalMapper.java Sep 29, 2023
@mimaison
Copy link
Member

In this case & is treated as logical AND. I think the only difference with && is that & always executes the right side expression even if the left side expression is false. So in this case I'm not sure you can write a test that differentiates & and &&.

@divijvaidya
Copy link
Contributor

In this case & is treated as logical AND. I think the only difference with && is that & always executes the right side expression even if the left side expression is false. So in this case I'm not sure you can write a test that differentiates & and &&.

You are right. I was mistaken in thinking that this is a bug.

@mimaison mimaison merged commit 03259f6 into apache:trunk Sep 29, 2023
rreddy-22 pushed a commit to rreddy-22/kafka-rreddy that referenced this pull request Oct 3, 2023
k-wall pushed a commit to k-wall/kafka that referenced this pull request Nov 21, 2023
@rykovsi rykovsi deleted the patch-1 branch December 29, 2023 13:09
AnatolyPopov pushed a commit to aiven/kafka that referenced this pull request Feb 16, 2024
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.

3 participants