-
Notifications
You must be signed in to change notification settings - Fork 15
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
create server with option host 'localhost' crash the database #38
Comments
Thanks, @cobolbaby ! Really
influxdb_fdw/sql/parameters_go.conf Line 2 in c7cc9f2
|
This is partially to influx-cxx https://github.com/pgspider/influxdb-cxx , see https://github.com/pgspider/influxdb-cxx/blob/5f2440d19ae457c53c0bbdb9337b6c10bd10e2cc/src/InfluxDBFactory.cxx#L65 As we know, InfluxDB metadata usage isn't very good implemented in this FDW. Can you try to improve this in C and C++? I can help with testing environment getting. |
If I wanna do more complete exception handling in influxdb_fdw, which piece of code needs to be modified? |
In my opinion option value validating Lines 106 to 107 in c7cc9f2
journal_mode operations in option.c https://github.com/pgspider/sqlite_fdw/pull/67/files#diff-2241a1312d5b2aced309a893bb6e00ce87796229a454eccbd65fbdb6b4ac6912 and something about partial string comparing in C. You can check if some initial characters is http:// or https:// and invalidate option otherwise before any FDW operations. Hence no need care about influx-cxx input values. Also please don't forget to write a test for invalid value near tests of valid values and follow existed PostgreSQL indent style.
|
@cobolbaby here is one of prefix compare functions, you can ensure it works as expected and use. bool check_prefix(const char *str, const char *prefix)
{
return strncmp(prefix, str, strlen(prefix)) == 0;
} |
Fixed in 054821e#diff-2241a1312d5b2aced309a893bb6e00ce87796229a454eccbd65fbdb6b4ac6912 by @jopoly. Please verify @cobolbaby and close this issue if all is ok with |
@mkgrgis It works. ALTER SERVER influxdb_svr
OPTIONS (SET host 'infra-influx.itc.inventec.net');
ERROR: influxdb_fdw: Host address must start with either http:// or https://
SQL state: XX000 |
It seems that the option
host
should be prefixed withhttp://
orhttps://
. Without the protocol header, the select statement on foreign_table will crash the postgresql 12.x , and you would get the error as below:I would like to ask if it is possible to verify the option when creating the server to avoid illegal format. Even if the format is illegal, it is better to give an error message when executing the query rather than crashing the database.
The text was updated successfully, but these errors were encountered: