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

Server version check #84

Merged
merged 6 commits into from Dec 18, 2018
Merged

Conversation

bpintea
Copy link
Collaborator

@bpintea bpintea commented Dec 18, 2018

This PR adds version checks of version strings.
If Elasticsearch'es version doesn't match own version, the connection is refused.

It also updates the reporting of the scalar functions the data source supports.

The driver will now report the qualifer in the version string.
- update the defines reporting data source's scalar capabilities.
The server and driver versions will be checked to match. It's thus safe
to report own version.
Check if server's and driver's versions are in sync.
Refuse connection if they aren't.
Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

I left a couple of suggestions but feel free to leave for a followup if you want to merge before feature freeze.

@@ -48,7 +48,7 @@ static SQLRETURN fake_answer(SQLHSTMT hstmt, const char *src, size_t cnt)
{
char *dup;

if (! (dup = strdup(src))) {
if (! (dup = _strdup(src))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This caters to Microsoft’s pedantry, but will make porting to Linux harder. I think an alternative is to /D_CRT_NONSTDC_NO_DEPRECATE.

driver/connect.c Outdated
static const wchar_t err_msg_fmt[] = L"Version mismatch between server ("
WPFWP_LDESC ") and driver (" WPFWP_LDESC "). Please use a driver whose"
" version matches that of your server.";
/* 32: max lenght of the version strings for which the explicit message
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: lenght

@bpintea
Copy link
Collaborator Author

bpintea commented Dec 18, 2018

Thank you for the prompt review! Comments addressed.

@bpintea bpintea merged commit 1c276a5 into elastic:master Dec 18, 2018
bpintea added a commit that referenced this pull request Dec 18, 2018
* append version qualifer to version string

The driver will now report the qualifer in the version string.

* update advertised function caps

- update the defines reporting data source's scalar capabilities.

* report own version when app is asking for DB ver.

The server and driver versions will be checked to match. It's thus safe
to report own version.

* fix compilation warnings (x86)

* check server's version against own

Check if server's and driver's versions are in sync.
Refuse connection if they aren't.

* revert to deprecated strdup, add warning silencer

- also fix a comment typo

(cherry picked from commit 1c276a5)
@bpintea bpintea deleted the feature/version_check branch December 18, 2018 23:33
@bpintea bpintea added >feature Applicable to PRs adding new functionality and removed >enhancement labels May 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>feature Applicable to PRs adding new functionality v6.6.0 v6.7.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants