summaryrefslogtreecommitdiff
path: root/src/interfaces
AgeCommit message (Collapse)Author
2024-10-23Fix incorrect struct reference in commentDaniel Gustafsson
SASL frontend mechanisms are implemented with pg_fe_sasl_mech and not the _be_ variant which is the backend implementation. Spotted while reading adjacent code.
2024-10-23ecpg: Fix out-of-bound read in DecodeDateTime()Michael Paquier
It was possible for the code to read out-of-bound data from the "day_tab" table with some crafted input data. Let's treat these as invalid input as the month number is incorrect. A test is added to test this case with a check on the errno returned by the decoding routine. A test close to the new one added in this commit was testing for a failure, but did not look at the errno generated, so let's use this commit to also change it, adding a check on the errno returned by DecodeDateTime(). Like the other test scripts, dt_test should likely be expanded to include more checks based on the errnos generated in these code paths. This is left as future work. This issue exists since 2e6f97560a83, so backpatch all the way down. Reported-by: Pavel Nekrasov Author: Bruce Momjian, Pavel Nekrasov Discussion: https://postgr.es/m/18614-6bbe00117352309e@postgresql.org Backpatch-through: 12
2024-10-22ecpg: Refactor ecpg_log() to skip unnecessary calls to ECPGget_sqlca().Fujii Masao
Previously, ecpg_log() always called ECPGget_sqlca() to retrieve sqlca, even though it was only needed for debug logging. This commit updates ecpg_log() to call ECPGget_sqlca() only when debug logging is enabled. Author: Yuto Sasaki Reviewed-by: Alvaro Herrera, Tom Lane, Fujii Masao Discussion: https://postgr.es/m/TY2PR01MB3628A85689649BABC9A1C6C3C1782@TY2PR01MB3628.jpnprd01.prod.outlook.com
2024-10-17ecpg: fix more minor mishandling of bad input in preprocessor.Tom Lane
Don't get confused by an unmatched right brace in the input. (Previously, this led to discarding information about file-level variables and then possibly crashing.) Detect, rather than crash on, an attempt to index into a non-array variable. As before, in the absence of field complaints I'm not too excited about back-patching these. Per valgrind testing by Alexander Lakhin. Discussion: https://postgr.es/m/a239aec2-6c79-5fc9-9272-cea41158a360@gmail.com
2024-10-16ecpg: fix some minor mishandling of bad input in preprocessor.Tom Lane
Avoid null-pointer crash when considering a cursor declaration that's outside any C function (a case which is useless anyway). Ensure a cursor for a prepared statement is marked as initially not open. At worst, if we chanced to get not-already-zeroed memory from malloc(), this oversight would result in failing to issue a "cursor "foo" has been declared but not opened" warning that would have been appropriate. Avoid running off the end of the buffer when there are mismatched square brackets following a variable name. This could lead to SIGSEGV after reaching the end of memory. Given the lack of field complaints, none of these seem to be worth back-patching, but let's clean them up in HEAD. Per valgrind testing by Alexander Lakhin. Discussion: https://postgr.es/m/5f5bcecd-d7ec-b8c0-6c92-d1a7c6e0f639@gmail.com
2024-10-14ecpg: invent a saner syntax for ecpg.addons entries.Tom Lane
Put the rule type at the start not the end, and put spaces between the constitutent token names instead of smashing them into an illegible mess. This has no functional impact but I think it makes the rules a great deal more readable. Discussion: https://postgr.es/m/1185216.1724001216@sss.pgh.pa.us
2024-10-14ecpg: add cross-checks to parse.pl for usage of internal tables.Tom Lane
parse.pl contains several constant tables that describe tweaks to be made to the backend grammar. In the same spirit as 00b0e7204, add cross-checks that each table entry is used at least once (or exactly once if that's appropriate). This should help catch cases where adjustments to the backend grammar cause a table entry not to match as expected. Per suggestion from Michael Paquier. Discussion: https://postgr.es/m/ZsLVbjsc5x5Saesg@paquier.xyz
2024-10-14ecpg: avoid breaking the IDENT precedence level in two.Tom Lane
Careless string hacking caused parse.pl to transform gram.y's declaration %nonassoc IDENT PARTITION RANGE ROWS ... into %nonassoc IDENT %nonassoc CSTRING PARTITION RANGE ROWS ... It turns out that this has no semantic impact, because the generated preproc.c is exactly the same either way (if you inject a blank line to keep line numbers the same). Nonetheless, given the great emphasis that the commentary in gram.y places on keeping those other keywords at the same precedence level as IDENT, this seems like foolishly risking ecpg behaving differently from the core parser. Adjust the code so that CSTRING is added to the precedence line without breaking it into two lines. Discussion: https://postgr.es/m/2157151.1713540065@sss.pgh.pa.us
2024-10-14ecpg: improve preprocessor's memory management.Tom Lane
Invent a notion of "local" storage that will automatically be reclaimed at the end of each statement. Use this for location strings as well as other visibly short-lived data within the parser. Also, make cat_str and make_str return local storage and not free their inputs, which allows dispensing with a whole lot of retail mm_strdup calls. We do have to add some new ones in places where a local-lifetime string needs to be added to a longer-lived data structure, but on balance there are a lot less mm_strdup calls than before. In hopes of flushing out places where changes were necessary, I changed YYLTYPE from "char *" to "const char *", which forced const-ification of various function arguments that probably should've been like that all along. This still leaks somewhat more memory than v17, but that will be cleaned up in future commits. Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
2024-10-14ecpg: move some functions into a new file ecpg/preproc/util.c.Tom Lane
mm_alloc and mm_strdup were in type.c, which seems a completely random choice. No doubt the original author thought two small functions didn't deserve their own file. But I'm about to add some more memory-management stuff beside them, so let's put them in a less surprising place. This seems like a better home for mmerror, mmfatal, and the cat_str/make_str family, too. Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
2024-10-14ecpg: re-implement preprocessor's string management.Tom Lane
Most productions in the preprocessor grammar construct strings representing SQL or C statements or fragments thereof. Instead of returning these as <str> results of the productions, return them as "location" values, taking advantage of Bison's flexibility about what a location is. We aren't really giving up anything thereby, since ecpg's error reports have always just given line numbers, and that's tracked separately. The advantage of this is that a single instance of the YYLLOC_DEFAULT macro can perform all the work needed by the vast majority of productions, including all the ones made automatically by parse.pl. This avoids having large numbers of effectively-identical productions, which tickles an optimization inefficiency in recent versions of clang. (This patch reduces the compilation time for preproc.o by more than 100-fold with clang 16, and is visibly helpful with gcc too.) The compiled parser is noticeably smaller as well. A disadvantage of this approach is that YYLLOC_DEFAULT is applied before running the production's semantic action (if any). This means it cannot use the method favored by cat_str() of free'ing all the input strings; if the action needs to look at the input strings, it'd be looking at dangling storage. As this stands, therefore, it leaks memory like a sieve. This is already a big patch though, and fixing the memory management seems like a separable problem, so let's leave that for the next step. (This does remove some free() calls that I'd have had to touch anyway, in the expectation that the next step will manage memory reclamation quite differently.) Most of the changes here are mindless substitution of "@N" for "$N" in grammar rules; see the changes to README.parser for an explanation. Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
2024-10-14ecpg: major cleanup, simplification, and documentation of parse.pl.Tom Lane
Remove a lot of cruft, clean up and document what's left. This produces the same preproc.y output as before, except for fewer blank lines. (It's not like we're making any attempt to match the layout of gram.y, so I removed the one bit of logic that seemed to have that in mind.) Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
2024-10-14ecpg: remove check_rules.pl.Tom Lane
As noted in the previous commit, check_rules.pl is now entirely redundant with checks made by parse.pl, or would be if it weren't for the places where it's wrong. It's a waste of build cycles and maintenance effort, so remove it. Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
2024-10-14ecpg: clean up documentation of parse.pl, and add more input checking.Tom Lane
README.parser is the user's manual, such as it is, for parse.pl. It's rather poorly written if you ask me; so try to improve it. (More could be written here, but this at least covers the same info in a more organized fashion.) Also, the single solitary line of usage info in parse.pl itself was a lie. Replace. Add some error checks that the ecpg.addons entries meet the syntax rules set forth in README.parser. One of them didn't, but accidentally worked anyway because the logic in include_addon is such that 'block' is the default behavior. Also add a cross-check that each ecpg.addons entry is matched exactly once in the backend grammar. This exposed that there are two dead entries there --- they are dead because the %replace_types table in parse.pl causes their nonterminals to be ignored altogether. Removing them doesn't change the generated preproc.y file. (This implies that check_rules.pl is completely worthless and should be nuked: it adds build cycles and maintenance effort while failing to reliably accomplish its one job of detecting dead rules. I'll do that separately.) Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
2024-10-14Remove traces of BeOS.Peter Eisentraut
Commit 15abc7788e6 tolerated namespace pollution from BeOS system headers. Commit 44f902122 de-supported BeOS. Since that stuff didn't make it into the Meson build system, synchronize by removing from configure. Author: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Japin Li <japinli@hotmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> (the idea, not the patch) Discussion: https://postgr.es/m/ME3P282MB3166F9D1F71F787929C0C7E7B6312%40ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM
2024-10-11Avoid mixing custom and OpenSSL BIO functionsDaniel Gustafsson
PostgreSQL has for a long time mixed two BIO implementations, which can lead to subtle bugs and inconsistencies. This cleans up our BIO by just just setting up the methods we need. This patch does not introduce any functionality changes. The following methods are no longer defined due to not being needed: - gets: Not used by libssl - puts: Not used by libssl - create: Sets up state not used by libpq - destroy: Not used since libpq use BIO_NOCLOSE, if it was used it close the socket from underneath libpq - callback_ctrl: Not implemented by sockets The following methods are defined for our BIO: - read: Used for reading arbitrary length data from the BIO. No change in functionality from the previous implementation. - write: Used for writing arbitrary length data to the BIO. No change in functionality from the previous implementation. - ctrl: Used for processing ctrl messages in the BIO (similar to ioctl). The only ctrl message which matters is BIO_CTRL_FLUSH used for writing out buffered data (or signal EOF and that no more data will be written). BIO_CTRL_FLUSH is mandatory to implement and is implemented as a no-op since there is no intermediate buffer to flush. BIO_CTRL_EOF is the out-of-band method for signalling EOF to read_ex based BIO's. Our BIO is not read_ex based but someone could accidentally call BIO_CTRL_EOF on us so implement mainly for completeness sake. As the implementation is no longer related to BIO_s_socket or calling SSL_set_fd, methods have been renamed to reference the PGconn and Port types instead. This also reverts back to using BIO_set_data, with our fallback, as a small optimization as BIO_set_app_data require the ex_data mechanism in OpenSSL. Author: David Benjamin <davidben@google.com> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/CAF8qwaCZ97AZWXtg_y359SpOHe+HdJ+p0poLCpJYSUxL-8Eo8A@mail.gmail.com
2024-10-06libpq: Discard leading and trailing spaces for parameters and values in URIsMichael Paquier
Integer values applied a parsing rule through pqParseIntParam() that made URIs like this one working, even if these include spaces around values: "postgresql://localhost:5432/postgres?keepalives=1 &keepalives_idle=1 " This commit changes the parsing so as spaces before and after parameters and values are discarded, offering more consistency with the parsing that already applied to libpq for integer values in URIs. Note that %20 can be used in a URI for a space character. ECPGconnect() has been discarded leading and trailing spaces around parameters and values that for a long time, as well. Like f22e84df1dea, this is done as a HEAD-only change. Reviewed-by: Yuto Sasaki Discussion: https://postgr.es/m/Zv3oWOfcrHTph7JK@paquier.xyz
2024-10-04ecpg: avoid adding whitespace around '&' in connection URLs.Tom Lane
The preprocessor really should not have done this to begin with. The space after '&' was masked by ECPGconnect's skipping spaces before option keywords, and the space before by dint of libpq being (mostly) insensitive to trailing space in option values. We fixed the one known problem with that in 920d51979. Hence this patch is mostly cosmetic, and we'll just change it in HEAD. Discussion: https://postgr.es/m/TY2PR01MB36286A7B97B9A15793335D18C1772@TY2PR01MB3628.jpnprd01.prod.outlook.com
2024-10-02Parse libpq's "keepalives" option more like other integer options.Tom Lane
Use pqParseIntParam (nee parse_int_param) instead of using strtol directly. This allows trailing whitespace, which the previous coding didn't, and makes the spelling of the error message consistent with other similar cases. This seems to be an oversight in commit e7a221797, which introduced parse_int_param. That fixed places that were using atoi(), but missed this place which was randomly using strtol() instead. Ordinarily I'd consider this minor cleanup not worth back-patching. However, it seems that ecpg assumes it can add trailing whitespace to URL parameters, so that use of the keepalives option fails in that context. Perhaps that's worth improving as a separate matter. In the meantime, back-patch this to all supported branches. Yuto Sasaki (some further cleanup by me) Discussion: https://postgr.es/m/TY2PR01MB36286A7B97B9A15793335D18C1772@TY2PR01MB3628.jpnprd01.prod.outlook.com
2024-10-01Simplify checking for xlocale.hPeter Eisentraut
Instead of XXX_IN_XLOCALE_H for several features XXX, let's just include <xlocale.h> if HAVE_XLOCALE_H. The reason for the extra complication was apparently that some old glibc systems also had an <xlocale.h>, and you weren't supposed to include it directly, but it's gone now (as far as I can tell it was harmless to do so anyway). Author: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech
2024-09-29In passwordFromFile, don't leak the open file after stat failures.Tom Lane
Oversight in e882bcae0. Per Coverity.
2024-09-09Add PQfullProtocolVersion() to surface the precise protocol version.Robert Haas
The existing function PQprotocolVersion() does not include the minor version of the protocol. In preparation for pending work that will bump that number for the first time, add a new function to provide it to clients that may care, using the (major * 10000 + minor) convention already used by PQserverVersion(). Jacob Champion based on earlier work by Jelte Fennema-Nio Discussion: http://postgr.es/m/CAOYmi+mM8+6Swt1k7XsLcichJv8xdhPnuNv7-02zJWsezuDL+g@mail.gmail.com
2024-09-08Avoid core dump after getpwuid_r failure.Tom Lane
Looking up a nonexistent user ID would lead to a null pointer dereference. That's unlikely to happen here, but perhaps not impossible. Thinko in commit 4d5111b3f, noticed by Coverity.
2024-09-05Prevent mis-encoding of "trailing junk after numeric literal" errors.Tom Lane
Since commit 2549f0661, we reject an identifier immediately following a numeric literal (without separating whitespace), because that risks ambiguity with hex/octal/binary integers. However, that patch used token patterns like "{integer}{ident_start}", which is problematic because {ident_start} matches only a single byte. If the first character after the integer is a multibyte character, this ends up with flex reporting an error message that includes a partial multibyte character. That can cause assorted bad-encoding problems downstream, both in the report to the client and in the postmaster log file. To fix, use {identifier} not {ident_start} in the "junk" token patterns, so that they will match complete multibyte characters. This seems generally better user experience quite aside from the encoding problem: for "123abc" the error message will now say that the error appeared at or near "123abc" instead of "123a". While at it, add some commentary about why these patterns exist and how they work. Report and patch by Karina Litskevich; review by Pavel Borisov. Back-patch to v15 where the problem came in. Discussion: https://postgr.es/m/CACiT8iZ_diop=0zJ7zuY3BXegJpkKK1Av-PU7xh0EDYHsa5+=g@mail.gmail.com
2024-09-05Check availability of module injection_points in TAP testsMichael Paquier
This fixes defects with installcheck for TAP tests that expect the module injection_points to exist in an installation, but the contents of src/test/modules are not installed by default with installcheck. This would cause, for example, failures under installcheck-world for a build with injection points enabled, when the contents of src/test/modules/ are not installed. The availability of the module can be done with a scan of pg_available_extension. This has been introduced in 2cdcae9da696, and it is refactored here as a new routine in Cluster.pm. Tests are changed in different ways depending on what they need: - The libpq TAP test sets up a node even without injection points, so it is enough to check that CREATE EXTENSION can be used. There is no need for the variable enable_injection_points. - In test_misc, 006_signal_autovacuum requires a runtime check. - 041_checkpoint_at_promote in recovery tests and 005_timeouts in test_misc are updated to use the routine introduced in Cluster.pm. - test_slru's 001_multixact, injection_points's 001_stats and modules/gin/ do not require a check as these modules disable installcheck entirely. Discussion: https://postgr.es/m/ZtesYQ-WupeAK7xK@paquier.xyz
2024-09-03Fix typos in code comments and test dataDaniel Gustafsson
The typos in 005_negotiate_encryption.pl and pg_combinebackup.c shall be backported to v17 where they were introduced. Backpatch-through: v17 Discussion: https://postgr.es/m/Ztaj7BkN4658OMxF@paquier.xyz
2024-09-03Fix typos and grammar in code comments and docsMichael Paquier
Author: Alexander Lakhin Discussion: https://postgr.es/m/f7e514cf-2446-21f1-a5d2-8c089a6e2168@gmail.com
2024-09-02Remove support for OpenSSL older than 1.1.0Daniel Gustafsson
OpenSSL 1.0.2 has been EOL from the upstream OpenSSL project for some time, and is no longer the default OpenSSL version with any vendor which package PostgreSQL. By retiring support for OpenSSL 1.0.2 we can remove a lot of no longer required complexity for managing state within libcrypto which is now handled by OpenSSL. Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/ZG3JNursG69dz1lr@paquier.xyz Discussion: https://postgr.es/m/CA+hUKGKh7QrYzu=8yWEUJvXtMVm_CNWH1L_TLWCbZMwbi1XP2Q@mail.gmail.com
2024-09-02More use of getpwuid_r() directlyPeter Eisentraut
Remove src/port/user.c, call getpwuid_r() directly. This reduces some complexity and allows better control of the error behavior. For example, the old code would in some circumstances silently truncate the result string, or produce error message strings that the caller wouldn't use. src/port/user.c used to be called src/port/thread.c and contained various portability complications to support thread-safety. These are all obsolete, and all but the user-lookup functions have already been removed. This patch completes this by also removing the user-lookup functions. Also convert src/backend/libpq/auth.c to use getpwuid_r() for thread-safety. Originally, I tried to be overly correct by using sysconf(_SC_GETPW_R_SIZE_MAX) to get the buffer size for getpwuid_r(), but that doesn't work on FreeBSD. All the OS where I could find the source code internally use 1024 as the suggested buffer size, so I just ended up hardcoding that. The previous code used BUFSIZ, which is an unrelated constant from stdio.h, so its use seemed inappropriate. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/flat/5f293da9-ceb4-4937-8e52-82c25db8e4d3%40eisentraut.org
2024-08-23Provide feature-test macros for libpq features added in v17.Tom Lane
As per the policy established in commit 6991e774e, invent macros that can be tested at compile time to detect presence of new libpq features. This should make calling code more readable and less error-prone than checking the libpq version would be (especially since we don't expose that at compile time; the server version is an unreliable substitute). Discussion: https://postgr.es/m/2042418.1724346970@sss.pgh.pa.us
2024-08-23thread-safety: gmtime_r(), localtime_r()Peter Eisentraut
Use gmtime_r() and localtime_r() instead of gmtime() and localtime(), for thread-safety. There are a few affected calls in libpq and ecpg's libpgtypes, which are probably effectively bugs, because those libraries already claim to be thread-safe. There is one affected call in the backend. Most of the backend otherwise uses the custom functions pg_gmtime() and pg_localtime(), which are implemented differently. While we're here, change the call in the backend to gmtime*() instead of localtime*(), since for that use time zone behavior is irrelevant, and this side-steps any questions about when time zones are initialized by localtime_r() vs localtime(). Portability: gmtime_r() and localtime_r() are in POSIX but are not available on Windows. Windows has functions gmtime_s() and localtime_s() that can fulfill the same purpose, so we add some small wrappers around them. (Note that these *_s() functions are also different from the *_s() functions in the bounds-checking extension of C11. We are not using those here.) On MinGW, you can get the POSIX-style *_r() functions by defining _POSIX_C_SOURCE appropriately before including <time.h>. This leads to a conflict at least in plpython because apparently _POSIX_C_SOURCE gets defined in some header there, and then our replacement definitions conflict with the system definitions. To avoid that sort of thing, we now always define _POSIX_C_SOURCE on MinGW and use the POSIX-style functions here. Reviewed-by: Stepan Neretin <sncfmgg@gmail.com> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/eba1dc75-298e-4c46-8869-48ba8aad7d70@eisentraut.org
2024-08-16libpq: Trace all messages received from the serverAlvaro Herrera
Not all messages that libpq received from the server would be sent through our message tracing logic. This commit tries to fix that by introducing a new function pqParseDone which make it harder to forget about doing so. The messages that we now newly send through our tracing logic are: - CopyData (received by COPY TO STDOUT) - Authentication requests - NegotiateProtocolVersion - Some ErrorResponse messages during connection startup - ReadyForQuery when received after a FunctionCall message Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS+YWXBhOGo+Y1YecLgknF3g@mail.gmail.com
2024-08-16libpq: Fix minor TOCTOU violationPeter Eisentraut
libpq checks the permissions of the password file before opening it. The way this is done in two separate operations, a static analyzer would flag as a time-of-check-time-of-use violation. In practice, you can't do anything with that, but it still seems better style to fix it. To fix it, open the file first and then check the permissions on the opened file handle. Reviewed-by: Aleksander Alekseev <aleksander@timescale.com> Reviewed-by: Andreas Karlsson <andreas@proxel.se> Discussion: https://www.postgresql.org/message-id/flat/a3356054-14ae-4e7a-acc6-249d19dac20b%40eisentraut.org
2024-08-15Remove dependence on -fwrapv semantics in a few places.Nathan Bossart
This commit attempts to update a few places, such as the money, numeric, and timestamp types, to no longer rely on signed integer wrapping for correctness. This is intended to move us closer towards removing -fwrapv, which may enable some compiler optimizations. However, there is presently no plan to actually remove that compiler option in the near future. Besides using some of the existing overflow-aware routines in int.h, this commit introduces and makes use of some new ones. Specifically, it adds functions that accept a signed integer and return its absolute value as an unsigned integer with the same width (e.g., pg_abs_s64()). It also adds functions that accept an unsigned integer, store the result of negating that integer in a signed integer with the same width, and return whether the negation overflowed (e.g., pg_neg_u64_overflow()). Finally, this commit adds a couple of tests for timestamps near POSTGRES_EPOCH_JDATE. Author: Joseph Koshakow Reviewed-by: Tom Lane, Heikki Linnakangas, Jian He Discussion: https://postgr.es/m/CAAvxfHdBPOyEGS7s%2Bxf4iaW0-cgiq25jpYdWBqQqvLtLe_t6tw%40mail.gmail.com
2024-08-15Clean up indentation and whitespace inconsistencies in ecpg.Tom Lane
ecpg's lexer and parser files aren't normally processed by pgindent, and unsurprisingly there's a lot of code in there that doesn't really match project style. I spent some time running pgindent over the fragments of these files that are C code, and this is the result. This is in the same spirit as commit 30ed71e42, though apparently Peter used a different method for that one, since it didn't find these problems. Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
2024-08-14libpq: Trace responses to SSLRequest and GSSENCRequestAlvaro Herrera
Since these are single bytes instead of v2 or v3 messages they need custom tracing logic. These "messages" don't even have official names in the protocol specification, so I (Jelte) called them SSLResponse and GSSENCResponse here. Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS+YWXBhOGo+Y1YecLgknF3g@mail.gmail.com
2024-08-12libpq: Trace frontend authentication challengesAlvaro Herrera
If tracing was enabled during connection startup, these messages would previously be listed in the trace output as something like this: F 54 Unknown message: 70 mismatched message length: consumed 4, expected 54 With this commit their type and contents are now correctly listed: F 36 StartupMessage 3 0 "user" "foo" "database" "alvherre" F 54 SASLInitialResponse "SCRAM-SHA-256" 32 'n,,n=,r=nq5zEPR/VREHEpOAZzH8Rujm' F 108 SASLResponse 'c=biws,r=nq5zEPR/VREHEpOAZzH8RujmVtWZDQ8glcrvy9OMNw7ZqFUn,p=BBwAKe0WjSvigB6RsmmArAC+hwucLeuwJrR5C/HQD5M=' Author: Jelte Fennema-Nio <postgres@jeltef.nl> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS+YWXBhOGo+Y1YecLgknF3g@mail.gmail.com
2024-08-10Fix inappropriate uses of atol()Peter Eisentraut
Some code using atol() would not work correctly if sizeof(long)==4: - src/bin/pg_basebackup/pg_basebackup.c: Would miscount size of a tablespace over 2 TB. - src/bin/pg_basebackup/streamutil.c: Would truncate a timeline ID beyond INT32_MAX. - src/bin/pg_rewind/libpq_source.c: Would miscount size of files larger than 2 GB (but this currently cannot happen). Replace these with atoll(). In one case, the use of atol() did not result in incorrect behavior but seems inconsistent with related code: - src/interfaces/ecpg/ecpglib/execute.c: Gratuitous, since it processes a value from pg_type.typlen, which is int16. Replace this with atoi(). Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/flat/a52738ad-06bc-4d45-b59f-b38a8a89de49%40eisentraut.org
2024-08-09libpq: Trace StartupMessage/SSLRequest/GSSENCRequest correctlyAlvaro Herrera
libpq tracing via PQtrace would uselessly print the wrong thing for these types of messages. With this commit, their type and contents would be correctly listed. (This can be verified with PQconnectStart(), but we don't use that in libpq_pipeline, so I (Álvaro) haven't bothered to add any tests.) Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS+YWXBhOGo+Y1YecLgknF3g@mail.gmail.com
2024-08-08libpq: Add suppress argument to pqTraceOutputNcharAlvaro Herrera
In future commits we're going to trace authentication related messages. Some of these messages contain challenge bytes as part of a challenge-response flow. Since these bytes are different for every connection, we want to normalize them when the PQTRACE_REGRESS_MODE trace flag is set. This commit modifies pqTraceOutputNchar to take a suppress argument, which makes it possible to do so. Author: Jelte Fennema-Nio <postgres@jeltef.nl> Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS+YWXBhOGo+Y1YecLgknF3g@mail.gmail.com
2024-08-07Revert ECPG's use of pnstrdup()Peter Eisentraut
Commit 0b9466fce added a dependency on fe_memutils' pnstrdup() inside informix.c. This adds an exit() path in a library, which we don't want. (Unlike libpq, the ecpg libraries don't have an automated check for that, but it makes sense to keep them to a similar standard.) The ecpg code can already handle failure results from the *strdup() call by itself. Author: Jacob Champion <jacob.champion@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/CAOYmi+=pg=W5L1h=3MEP_EB24jaBu2FyATrLXqQHGe7cpuvwyg@mail.gmail.com
2024-08-03Add -Wmissing-variable-declarations to the standard compilation flagsPeter Eisentraut
This warning flag detects global variables not declared in header files. This is similar to what -Wmissing-prototypes does for functions. (More correctly, it is similar to what -Wmissing-declarations does for functions, but -Wmissing-prototypes is a superset of that in C.) This flag is new in GCC 14. Clang has supported it for a while. Several recent commits have cleaned up warnings triggered by this, so it should now be clean. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org
2024-08-02Include bison header files into implementation filesPeter Eisentraut
Before Bison 3.4, the generated parser implementation files run afoul of -Wmissing-variable-declarations (in spite of commit ab61c40bfa2) because declarations for yylval and possibly yylloc are missing. The generated header files contain an extern declaration, but the implementation files don't include the header files. Since Bison 3.4, the generated implementation files automatically include the generated header files, so then it works. To make this work with older Bison versions as well, include the generated header file from the .y file. (With older Bison versions, the generated implementation file contains effectively a copy of the header file pasted in, so including the header file is redundant. But we know this works anyway because the core grammar uses this arrangement already.) Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org
2024-08-01Convert some extern variables to static, Windows codePeter Eisentraut
Similar to 720b0eaae9b, discovered by MinGW.
2024-07-28libpq: Use strerror_r instead of strerrorPeter Eisentraut
Commit 453c4687377 introduced a use of strerror() into libpq, but that is not thread-safe. Fix by using strerror_r() instead. In passing, update some of the code comments added by 453c4687377, as we have learned more about the reason for the change in OpenSSL that started this. Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: Discussion: https://postgr.es/m/b6fb018b-f05c-4afd-abd3-318c649faf18@highgo.ca
2024-07-26Add tests for errors during SSL or GSSAPI handshakeHeikki Linnakangas
These test that libpq correctly falls back to a plaintext connection on handshake error, in the "prefer" modes. Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/CAOYmi%2Bnwvu21mJ4DYKUa98HdfM_KZJi7B1MhyXtnsyOO-PB6Ww%40mail.gmail.com
2024-07-26Add test for early backend startup errorsHeikki Linnakangas
The new test tests the libpq fallback behavior on an early error, which was fixed in the previous commit. This adds an IS_INJECTION_POINT_ATTACHED() macro, to allow writing injected test code alongside the normal source code. In principle, the new test could've been implemented by an extra test module with a callback that sets the FrontendProtocol global variable, but I think it's more clear to have the test code right where the injection point is, because it has pretty intimate knowledge of the surrounding context it runs in. Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/CAOYmi%2Bnwvu21mJ4DYKUa98HdfM_KZJi7B1MhyXtnsyOO-PB6Ww%40mail.gmail.com
2024-07-26Fix fallback behavior when server sends an ERROR early at startupHeikki Linnakangas
With sslmode=prefer, the desired behavior is to completely fail the connection attempt, *not* fall back to a plaintext connection, if the server responds to the SSLRequest with an error ('E') response instead of rejecting SSL with an 'N' response. This was broken in commit 05fd30c0e7. Reported-by: Jacob Champion Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/CAOYmi%2Bnwvu21mJ4DYKUa98HdfM_KZJi7B1MhyXtnsyOO-PB6Ww%40mail.gmail.com Backpatch-through: 17
2024-07-25Add extern declarations for Bison global variablesPeter Eisentraut
This adds extern declarations for some global variables produced by Bison that are not already declared in its generated header file. This is a workaround to be able to add -Wmissing-variable-declarations to the global set of warning options in the near future. Another longer-term solution would be to convert these grammars to "pure" parsers in Bison, to avoid global variables altogether. Note that the core grammar is already pure, so this patch did not need to touch it. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org
2024-07-08Fix outdated comment after removal of direct SSL fallbackHeikki Linnakangas
The option to fall back from direct SSL to negotiated SSL or a plaintext connection was removed in commit fb5718f35f. Discussion: https://www.postgresql.org/message-id/c82ad227-e049-4e18-8898-475a748b5a5a@iki.fi