Skip to content

Flip incorrect logic for dual backends #2762

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PrismaPhonic
Copy link

There was a piece of code in the client builder that has the following code comment:

"Default backend + rustls Identity doesn't work."

It appears however that this piece of code was being run when rustls is enabled, and native-tls is DISABLED, which seems to be a mistake. This changes the feature flag to only run when both rustls and the native backend are BOTH enabled.

This somehow got introduced recently - perhaps the code wasn't in use at all and a recent refactor caused it to actually be used, exposing the bug?

There was a piece of code in the client builder that has the following
code comment:

"Default backend + rustls Identity doesn't work."

It appears however that this piece of code was being run when rustls is
enabled, and native-tls is DISABLED, which seems to be a mistake. This
changes the feature flag to only run when both rustls and the native
backend are BOTH enabled.
@seanmonstar
Copy link
Owner

Thanks for the PR!

This somehow got introduced recently - perhaps the code wasn't in use at all and a recent refactor caused it to actually be used, exposing the bug?

How do you mean? This conditional was changed recently? Do you know when?

@PrismaPhonic
Copy link
Author

PrismaPhonic commented Jul 14, 2025

I would have to look through the code to figure out how this happened better. This specific conditional was committed two years ago, but only recently did we hit this conditional line of code in our codebase that uses reqwest. Regardless, unless I'm simply misunderstanding something here, the conditional was written incorrectly for what it was trying to enforce.

Am I misunderstanding something? This change ^ fixed our repo that is using reqwest and started breaking recently. We are only enabling the rustls-tls flag and have default features turned off, and we are hitting this error message. Am I incorrect and using native-tls is required when using the rustls flag?

@PrismaPhonic
Copy link
Author

I'm realizing now that I completely missed the cfg flag on the match line this is a part of - it makes a lot more sense now! I'm still not sure why the native-tls flag is a part of this conditional at all.

Having said that, it appears that after a recent cargo update, we somehow are now enabling the native tls backend, which seems odd, because as I said, we only have rustls-tls flag set with default features turned off. I'll investigate further and report back - apologies for the misleading PR

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