Skip to content

Implement #8066 : Make protocol schemes case-insensitive. #8097

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

Merged
merged 1 commit into from
Apr 28, 2024
Merged

Conversation

hvlad
Copy link
Member

@hvlad hvlad commented Apr 28, 2024

No description provided.

@hvlad hvlad self-assigned this Apr 28, 2024
@hvlad hvlad linked an issue Apr 28, 2024 that may be closed by this pull request
@aafemt
Copy link
Contributor

aafemt commented Apr 28, 2024

Wouldn't be clearer to declare prefix as NoCaseString and then use it's compare (in absence of starts_with from C++20)?

@hvlad
Copy link
Member Author

hvlad commented Apr 28, 2024

Wouldn't be clearer to declare prefix as NoCaseString and then use it's compare (in absence of starts_with from C++20)?

compare() takes into account string length and can not work as starts_with()

@aafemt
Copy link
Contributor

aafemt commented Apr 28, 2024

compare() takes into account string length and can not work as starts_with()

AFAIU this overload could work:
int compare(const_pointer s, const size_type n) const
Because c_str() is guaranteed to be zero-terminated, result of prefix.compare(expanded_name.c_str(), prefix.length()) should do exactly the same what your code does if prefix is NoCaseString.

@hvlad
Copy link
Member Author

hvlad commented Apr 28, 2024

compare() takes into account string length and can not work as starts_with()

AFAIU this overload could work: int compare(const_pointer s, const size_type n) const Because c_str() is guaranteed to be zero-terminated, result of prefix.compare(expanded_name.c_str(), prefix.length()) should do exactly the same what your code does if prefix is NoCaseString.

It could access memory out of expanded_name buffer, if expanded_name.length() < prefix.length()

If you offer to replace just one string (IgnoreCaseComparator::compare), I see no value in it at all as it definitely doesn't make code "clear" - it passes into compare() one string with length of another one.

@hvlad hvlad merged commit a9b806a into master Apr 28, 2024
@hvlad hvlad deleted the work/gh-8066 branch April 28, 2024 19:54
@aafemt
Copy link
Contributor

aafemt commented Apr 28, 2024

It could access memory out of expanded_name buffer, if expanded_name.length() < prefix.length()

Only if comparison is greedy and doesn't stop at the first different byte (which would be the expanded_name's terminating <NUL>).

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.

Make protocol schemes case-insensitive.
3 participants