From f8d51b4e857770a1977608615ddf1fac41424b5c Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 10 Jun 2026 11:13:27 +0100 Subject: [PATCH 1/5] Add Query Tool result export enhancements. Several long-standing requests around exporting/copying Query Tool results, all in the results download/copy path: - Save results as JSON or XML in addition to CSV, selectable from a drop-down on the "Save results to file" toolbar button. The download generator is now format-aware and streams JSON/XML as well as CSV. - New "Output file encoding" preference (CSV/TXT output) controlling the character encoding of saved results; defaults to utf-8. - New "Add byte order mark (BOM)?" preference that prepends a UTF BOM to saved CSV/TXT files for better interoperability with applications such as Microsoft Excel. - New "Copy with headers?" preference seeding the default state of the results grid "Copy with headers" toggle. Adds integration tests for the JSON/XML/BOM/encoding download paths and updates the preferences and Query Tool toolbar documentation. Closes #3205 Closes #4128 Closes #4129 Closes #6695 --- docs/en_US/preferences.rst | 10 ++ docs/en_US/query_tool_toolbar.rst | 10 +- web/pgadmin/tools/sqleditor/__init__.py | 59 ++++++-- .../js/components/sections/ResultSet.jsx | 14 +- .../components/sections/ResultSetToolbar.jsx | 28 +++- .../tests/test_download_csv_query_tool.py | 130 ++++++++++++++++++ .../sqleditor/utils/query_tool_preferences.py | 38 +++++ .../utils/driver/psycopg3/connection.py | 127 +++++++++++++---- 8 files changed, 364 insertions(+), 52 deletions(-) diff --git a/docs/en_US/preferences.rst b/docs/en_US/preferences.rst index 8342571ab5c..630d2070966 100644 --- a/docs/en_US/preferences.rst +++ b/docs/en_US/preferences.rst @@ -589,6 +589,13 @@ Use the fields on the *CSV/TXT Output* panel to control the CSV/TXT output. quoted in the CSV/TXT output; select *Strings*, *All*, or *None*. * Use the *Replace null values with* option to replace null values with specified string in the output file. Default is set to 'NULL'. +* Use the *Output file encoding* drop-down listbox to specify the character + encoding used when saving query results to a file. The default is utf-8; an + encoding that is not listed can also be typed in. +* Use the *Add byte order mark (BOM)?* switch to add a byte order mark at the + start of the saved file when a UTF encoding is used. This helps applications + such as Microsoft Excel detect the encoding correctly. This applies to the + CSV/TXT output only. .. image:: images/preferences_sql_display.png :alt: Preferences sqleditor display options @@ -754,6 +761,9 @@ preferences for copied data. character for copied data. * Use the *Result copy quoting* drop-down listbox to select which type of fields require quoting; select *All*, *None*, or *Strings*. +* When the *Copy with headers?* switch is set to true, the column headers are + included by default when copying data from the results grid. This can still + be toggled per-copy from the results grid copy options menu. * When the *Striped rows?* switch is set to true, the result grid will display rows with alternating background colors. diff --git a/docs/en_US/query_tool_toolbar.rst b/docs/en_US/query_tool_toolbar.rst index 9b03ca42ac9..848089a5659 100644 --- a/docs/en_US/query_tool_toolbar.rst +++ b/docs/en_US/query_tool_toolbar.rst @@ -210,10 +210,12 @@ Data Editing Options | *Save Data Changes* | Click the *Save Data Changes* icon to save data changes (insert, update, or delete) in the Data | F6 | | | Output Panel to the server. | | +----------------------+---------------------------------------------------------------------------------------------------+----------------+ - | *Save results to* | Click the Save results to file icon to save the result set of the current query as a delimited | F8 | - | *file* | text file (CSV, if the field separator is set to a comma). This button will only be enabled when | | - | | a query has been executed and there are results in the data grid. You can specify the CSV/TXT | | - | | settings in the Preference Dialogue under SQL Editor -> CSV/TXT output. | | + | *Save results to* | Click the Save results to file icon to save the result set of the current query. By | F8 | + | *file* | default it is saved as a delimited text file (CSV, if the field separator is set to a | | + | | comma). Use the adjacent drop-down list to instead save the results as JSON or XML. | | + | | This button is only enabled when a query has been executed and there are results in | | + | | the data grid. You can specify the CSV/TXT settings (including the output file encoding | | + | | and byte order mark) in the Preferences dialog under Query Tool -> CSV/TXT Output. | | +----------------------+---------------------------------------------------------------------------------------------------+----------------+ | Graph Visualiser | Use the Graph Visualiser button to generate graphs of the query results. | | +----------------------+---------------------------------------------------------------------------------------------------+----------------+ diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 8080d220a54..29901272984 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -2179,20 +2179,59 @@ def start_query_download_tool(trans_id): } ) + # Output format: csv (default), json or xml. + data_format = (data.get('format') or 'csv').lower() + if data_format not in ('csv', 'json', 'xml'): + data_format = 'csv' + + # Encoding and BOM apply to the CSV/text output only; the structured + # formats are always emitted as UTF-8. + if data_format == 'csv': + output_encoding = blueprint.csv_output_encoding.get() or 'utf-8' + add_bom = blueprint.csv_add_bom.get() + else: + output_encoding = 'utf-8' + add_bom = False + is_utf = output_encoding.lower().replace('-', '').replace( + '_', '').startswith('utf') + + str_gen = gen(conn_obj, + trans_obj, + quote=blueprint.csv_quoting.get(), + quote_char=blueprint.csv_quote_char.get(), + field_separator=blueprint.csv_field_separator.get(), + replace_nulls_with=blueprint.replace_nulls_with.get(), + data_format=data_format) + + def encoded_gen(text_gen): + is_first_chunk = True + for chunk in text_gen: + if is_first_chunk: + is_first_chunk = False + if add_bom and is_utf: + chunk = '\ufeff' + chunk + yield chunk.encode(output_encoding, errors='replace') + + if data_format == 'json': + base_mimetype = 'application/json' + elif data_format == 'xml': + base_mimetype = 'application/xml' + elif blueprint.csv_field_separator.get() == ',': + base_mimetype = 'text/csv' + else: + base_mimetype = 'text/plain' + r = Response( - gen(conn_obj, - trans_obj, - quote=blueprint.csv_quoting.get(), - quote_char=blueprint.csv_quote_char.get(), - field_separator=blueprint.csv_field_separator.get(), - replace_nulls_with=blueprint.replace_nulls_with.get()), - mimetype='text/csv' if - blueprint.csv_field_separator.get() == ',' - else 'text/plain' + encoded_gen(str_gen), + mimetype='{0}; charset={1}'.format(base_mimetype, output_encoding) ) import time - extn = 'csv' if blueprint.csv_field_separator.get() == ',' else 'txt' + if data_format == 'csv': + extn = 'csv' if blueprint.csv_field_separator.get() == ',' \ + else 'txt' + else: + extn = data_format filename = data['filename'] if data.get('filename', '') != "" else \ '{0}.{1}'.format(int(time.time()), extn) diff --git a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx index 639bd39598c..16ae2bd5d57 100644 --- a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx +++ b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSet.jsx @@ -476,7 +476,8 @@ export class ResultSetUtils { }); } - async saveResultsToFile(fileName, onProgress) { + async saveResultsToFile(fileName, onProgress, dataFormat='csv') { + const mimeTypes = {csv: 'text/csv', json: 'application/json', xml: 'application/xml'}; try { await DownloadUtils.downloadFileStream({ url: url_for('sqleditor.query_tool_download', { @@ -484,8 +485,8 @@ export class ResultSetUtils { }), options: { method: 'POST', - body: JSON.stringify({filename: fileName, query_commited: this.hasQueryCommitted}) - }}, fileName, 'text/csv', onProgress); + body: JSON.stringify({filename: fileName, query_commited: this.hasQueryCommitted, format: dataFormat}) + }}, fileName, mimeTypes[dataFormat] ?? 'text/csv', onProgress); this.eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS_END); } catch (error) { this.eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS_END); @@ -1052,8 +1053,9 @@ export function ResultSet() { setLoaderText(null); }); - eventBus.registerListener(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS, async ()=>{ - let extension = queryToolCtx.preferences?.sqleditor?.csv_field_separator === ',' ? '.csv': '.txt'; + eventBus.registerListener(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS, async (dataFormat='csv')=>{ + const csvExtension = queryToolCtx.preferences?.sqleditor?.csv_field_separator === ',' ? '.csv': '.txt'; + let extension = {csv: csvExtension, json: '.json', xml: '.xml'}[dataFormat] ?? csvExtension; let fileName = 'data-' + new Date().getTime() + extension; if(!queryToolCtx.params.is_query_tool) { fileName = queryToolCtx.params.node_name + extension; @@ -1061,7 +1063,7 @@ export function ResultSet() { setLoaderText(gettext('Downloading results...')); await rsu.current.saveResultsToFile(fileName, (p)=>{ setLoaderText(gettext('Downloading results(%s)...', p)); - }); + }, dataFormat); setLoaderText(''); }); diff --git a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSetToolbar.jsx b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSetToolbar.jsx index 793ca45dcc4..36daa8b1131 100644 --- a/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSetToolbar.jsx +++ b/web/pgadmin/tools/sqleditor/static/js/components/sections/ResultSetToolbar.jsx @@ -282,6 +282,7 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all /* Menu button refs */ const copyMenuRef = React.useRef(null); const pasetMenuRef = React.useRef(null); + const downloadMenuRef = React.useRef(null); const queryToolPref = queryToolCtx.preferences.sqleditor; @@ -309,8 +310,8 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all const addRow = useCallback(()=>{ eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_ADD_ROWS, [[]], {isNewRow: true}); }, []); - const downloadResult = useCallback(()=>{ - eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS); + const downloadResult = useCallback((fmt='csv')=>{ + eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_RESULTS, fmt); }, []); const showGraphVisualiser = useCallback(()=>{ eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_GRAPH_VISUALISER); @@ -348,6 +349,14 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all setDisableButton('save-result', (totalRowCount||0) < 1); }, [totalRowCount]); + useEffect(()=>{ + // Seed the "Copy with headers" toggle default from the user preference. + setCheckedMenuItems((prev)=>({ + ...prev, + copy_with_headers: queryToolPref.copy_column_headers, + })); + }, [queryToolPref.copy_column_headers]); + useEffect(()=>{ eventBus.registerListener(QUERY_TOOL_EVENTS.TRIGGER_COPY_DATA, copyData); return ()=>eventBus.deregisterListener(QUERY_TOOL_EVENTS.TRIGGER_COPY_DATA, copyData); @@ -432,7 +441,10 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all } - onClick={downloadResult} shortcut={queryToolPref.download_results} + onClick={()=>downloadResult('csv')} shortcut={queryToolPref.download_results} + disabled={buttonsDisabled['save-result']} /> + } splitButton + name="menu-downloadoptions" ref={downloadMenuRef} onClick={openMenu} disabled={buttonsDisabled['save-result']} /> @@ -490,6 +502,16 @@ export function ResultSetToolbar({query, canEdit, totalRowCount, pagination, all > {gettext('Paste with SERIAL/IDENTITY values?')} + + downloadResult('csv')}>{gettext('Save as CSV/Text')} + downloadResult('json')}>{gettext('Save as JSON')} + downloadResult('xml')}>{gettext('Save as XML')} + ); } diff --git a/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py b/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py index f4157dbdc7f..17a1bd28f44 100644 --- a/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py +++ b/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py @@ -240,3 +240,133 @@ def tearDown(self): self.server['sslmode'] ) test_utils.drop_database(main_conn, self._db_name) + + +class TestDownloadResultFormats(BaseTestGenerator): + """ + Validates downloading query results as JSON and XML, the UTF BOM option + and the output file encoding option. + """ + SQL = 'SELECT 1 as "A", 2 as "B", \'x\' as "C"' + INIT_URL = '/sqleditor/initialize/sqleditor/{0}/{1}/{2}/{3}' + DOWNLOAD_URL = '/sqleditor/query_tool/download/{0}' + + scenarios = [ + ( + 'Download results as JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json') + ), + ( + 'Download results as XML', + dict(data_format='xml', add_bom=False, encoding='utf-8', + expected_content_type='application/xml', + expected_extension='.xml') + ), + ( + 'Download CSV with a UTF BOM', + dict(data_format='csv', add_bom=True, encoding='utf-8', + expected_content_type='text/csv', + expected_extension='.csv') + ), + ( + 'Download CSV with a non-UTF output encoding', + dict(data_format='csv', add_bom=True, encoding='latin-1', + expected_content_type='text/csv', + expected_extension='.csv') + ), + ] + + def setUp(self): + self._db_name = 'download_results_fmt_' + str( + secrets.choice(range(10000, 65535))) + self._sid = self.server_information['server_id'] + server_utils.connect_server(self, self._sid) + self._did = test_utils.create_database(self.server, self._db_name) + + def initiate_sql_query_tool(self, trans_id, sql_query): + url = '/sqleditor/query_tool/start/{0}'.format(trans_id) + response = self.tester.post(url, data=json.dumps({"sql": sql_query}), + content_type='html/json') + self.assertEqual(response.status_code, 200) + return async_poll(tester=self.tester, + poll_url='/sqleditor/poll/{0}'.format(trans_id)) + + def runTest(self): + db_con = database_utils.connect_database(self, + test_utils.SERVER_GROUP, + self._sid, + self._did) + if not db_con["info"] == "Database connected.": + raise Exception("Could not connect to the database.") + + self.trans_id = str(secrets.choice(range(1, 9999999))) + url = self.INIT_URL.format( + self.trans_id, test_utils.SERVER_GROUP, self._sid, self._did) + response = self.tester.post(url, data=json.dumps({ + "dbname": self._db_name + })) + self.assertEqual(response.status_code, 200) + + self.initiate_sql_query_tool(self.trans_id, self.SQL) + + url = self.DOWNLOAD_URL.format(self.trans_id) + self.app.logger.disabled = True + filename = 'test{0}'.format(self.expected_extension) + with patch('pgadmin.tools.sqleditor.blueprint.' + 'csv_add_bom.get', return_value=self.add_bom), \ + patch('pgadmin.tools.sqleditor.blueprint.' + 'csv_output_encoding.get', return_value=self.encoding): + response = self.tester.post(url, data={ + "query": self.SQL, + "filename": filename, + "format": self.data_format, + "query_commited": True, + }) + self.app.logger.disabled = False + + headers = dict(response.headers) + self.assertEqual(response.status_code, 200) + self.assertIn(self.expected_content_type, headers['Content-Type']) + self.assertIn('charset={0}'.format(self.encoding), + headers['Content-Type']) + self.assertIn(filename, headers['Content-Disposition']) + + raw = response.data + if self.add_bom and self.encoding.lower().startswith('utf'): + self.assertTrue(raw.startswith(b'\xef\xbb\xbf')) + else: + self.assertFalse(raw.startswith(b'\xef\xbb\xbf')) + + body = raw.decode(self.encoding) + + if self.data_format == 'json': + parsed = json.loads(body) + self.assertIsInstance(parsed, list) + self.assertEqual(parsed[0]['A'], 1) + self.assertEqual(parsed[0]['B'], 2) + self.assertEqual(parsed[0]['C'], 'x') + elif self.data_format == 'xml': + self.assertIn('', body) + self.assertIn('1', body) + self.assertIn('x', body) + self.assertIn('', body) + else: + self.assertIn('"A","B","C"', body) + + url = '/sqleditor/close/{0}'.format(self.trans_id) + response = self.tester.delete(url) + self.assertEqual(response.status_code, 200) + database_utils.disconnect_database(self, self._sid, self._did) + + def tearDown(self): + main_conn = test_utils.get_db_connection( + self.server['db'], + self.server['username'], + self.server['db_password'], + self.server['host'], + self.server['port'], + self.server['sslmode'] + ) + test_utils.drop_database(main_conn, self._db_name) diff --git a/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py b/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py index 5b6e8941f24..a037b833707 100644 --- a/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py +++ b/web/pgadmin/tools/sqleditor/utils/query_tool_preferences.py @@ -307,6 +307,34 @@ def register_query_tool_preferences(self): allow_blanks=True ) + self.csv_output_encoding = self.preference.register( + 'CSV_output', 'csv_output_encoding', + gettext("Output file encoding"), 'options', 'utf-8', + category_label=PREF_LABEL_CSV_TXT, + options=[{'label': 'utf-8', 'value': 'utf-8'}, + {'label': 'utf-16', 'value': 'utf-16'}, + {'label': 'latin-1', 'value': 'latin-1'}, + {'label': 'windows-1252', 'value': 'windows-1252'}], + control_props={ + 'allowClear': False, + 'tags': False, + 'creatable': True + }, + help_str=gettext('The character encoding used when saving query ' + 'results to a file. Defaults to utf-8. A different ' + 'encoding can be typed in if it is not listed.') + ) + + self.csv_add_bom = self.preference.register( + 'CSV_output', 'csv_add_bom', + gettext("Add byte order mark (BOM)?"), 'boolean', + False, category_label=PREF_LABEL_CSV_TXT, + help_str=gettext('If set to True, a byte order mark (BOM) is added at ' + 'the start of the saved file when a UTF encoding is ' + 'used. This helps applications such as Microsoft ' + 'Excel detect the encoding correctly.') + ) + self.results_grid_quoting = self.preference.register( 'Results_grid', 'results_grid_quoting', gettext("Result copy quoting"), 'options', 'strings', @@ -347,6 +375,16 @@ def register_query_tool_preferences(self): } ) + self.copy_column_headers = self.preference.register( + 'Results_grid', 'copy_column_headers', + gettext("Copy with headers?"), 'boolean', + False, category_label=PREF_LABEL_RESULTS_GRID, + help_str=gettext('If set to True, the column headers are included by ' + 'default when copying data from the results grid. ' + 'This can still be toggled per-copy from the results ' + 'grid copy menu.') + ) + self.column_data_auto_resize = self.preference.register( 'Results_grid', 'column_data_auto_resize', gettext("Columns sized by"), 'radioModern', 'by_data', diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index d07a16cefcd..deb2867ea8e 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -17,7 +17,9 @@ import secrets import datetime import asyncio +import json from collections import deque +from xml.sax.saxutils import escape as xml_escape, quoteattr as xml_quoteattr import psycopg from flask import g, current_app from flask_babel import gettext @@ -54,6 +56,62 @@ _ = gettext + +def _json_default(value): + """Fallback serialiser for values that json cannot encode natively + (dates, Decimals, intervals, etc.).""" + return str(value) + + +def _generate_json(cur, records, results, header, replace_nulls_with, + handle_null_values): + """Stream the result set as a JSON array of row objects. + + The first batch of rows (``results``) has already been fetched by the + caller; subsequent batches are pulled with ``fetchmany(records)``. + """ + yield '[' + is_first_row = True + while results: + if replace_nulls_with is not None: + results = handle_null_values(results, replace_nulls_with) + for row in results: + row_json = json.dumps(dict(row), default=_json_default) + yield row_json if is_first_row else ',' + row_json + is_first_row = False + results = cur.fetchmany(records) + yield ']' + + +def _generate_xml(cur, records, results, header, replace_nulls_with, + handle_null_values): + """Stream the result set as XML. + + Column names are emitted as escaped ``name`` attributes (rather than + element names) so that column names which are not valid XML element + names are handled safely. + """ + yield '\n' + while results: + if replace_nulls_with is not None: + results = handle_null_values(results, replace_nulls_with) + for row in results: + row_io = [''] + for column in header: + value = row.get(column) + if value is None: + row_io.append( + ''.format( + xml_quoteattr(column))) + else: + row_io.append('{1}'.format( + xml_quoteattr(column), xml_escape(str(value)))) + row_io.append('') + yield ''.join(row_io) + results = cur.fetchmany(records) + yield '' + + # Register global type caster which will be applicable to all connections. register_global_typecasters() configure_driver_encodings(encodings) @@ -927,7 +985,8 @@ def handle_null_values(results, replace_nulls_with): return results def gen(conn_obj, trans_obj, quote='strings', quote_char="'", - field_separator=',', replace_nulls_with=None): + field_separator=',', replace_nulls_with=None, + data_format='csv'): try: cur.scroll(0, mode='absolute') @@ -950,37 +1009,24 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", if c.to_dict()['type_code'] in ALL_JSON_TYPES: json_columns.append(column_name) - res_io = StringIO() - - if quote == 'strings': - quote = csv.QUOTE_NONNUMERIC - elif quote == 'all': - quote = csv.QUOTE_ALL + if data_format == 'json': + yield from _generate_json(cur, records, results, header, + replace_nulls_with, + handle_null_values) + elif data_format == 'xml': + yield from _generate_xml(cur, records, results, header, + replace_nulls_with, + handle_null_values) else: - quote = csv.QUOTE_NONE - - csv_writer = csv.DictWriter( - res_io, fieldnames=header, delimiter=field_separator, - quoting=quote, - quotechar=quote_char, - replace_nulls_with=replace_nulls_with - ) - - csv_writer.writeheader() - # Replace the null values with given string if configured. - if replace_nulls_with is not None: - results = handle_null_values(results, replace_nulls_with) - csv_writer.writerows(results) - - yield res_io.getvalue() - - while True: - results = cur.fetchmany(records) - - if not results: - break res_io = StringIO() + if quote == 'strings': + quote = csv.QUOTE_NONNUMERIC + elif quote == 'all': + quote = csv.QUOTE_ALL + else: + quote = csv.QUOTE_NONE + csv_writer = csv.DictWriter( res_io, fieldnames=header, delimiter=field_separator, quoting=quote, @@ -988,12 +1034,35 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", replace_nulls_with=replace_nulls_with ) + csv_writer.writeheader() # Replace the null values with given string if configured. if replace_nulls_with is not None: results = handle_null_values(results, replace_nulls_with) csv_writer.writerows(results) + yield res_io.getvalue() + while True: + results = cur.fetchmany(records) + + if not results: + break + res_io = StringIO() + + csv_writer = csv.DictWriter( + res_io, fieldnames=header, delimiter=field_separator, + quoting=quote, + quotechar=quote_char, + replace_nulls_with=replace_nulls_with + ) + + # Replace the null values with given string if configured. + if replace_nulls_with is not None: + results = handle_null_values(results, + replace_nulls_with) + csv_writer.writerows(results) + yield res_io.getvalue() + try: # try to reset the cursor scroll back to where it was, # bypass error, if cannot scroll back From bcf68f410933958f739b3d2dfff559d0942e4b08 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Thu, 11 Jun 2026 10:41:34 +0100 Subject: [PATCH 2/5] Fix double BOM and validate output encoding for result export. Two fixes from code review of the Query Tool result export feature: - Avoid emitting a double byte-order mark for the 'utf-16' and 'utf-32' output encodings. Those codecs (without an explicit endianness suffix) self-emit a BOM, so hand-prepending another produced two BOMs and a corrupt file. We now only hand-write the BOM for codecs that do not emit one themselves (utf-8 and the explicit-endian utf-16/32-le/-be forms), guaranteeing exactly one BOM for every utf-* encoding. - Validate the user-configurable output encoding up front with codecs.lookup() before building the streaming Response, returning a clean 400 instead of raising LookupError mid-stream (which produced a truncated 200 with a raw traceback). Adds test scenarios asserting utf-16 output carries exactly one BOM and that an invalid encoding returns a 400. --- web/pgadmin/tools/sqleditor/__init__.py | 30 +++++++++++-- .../tests/test_download_csv_query_tool.py | 45 ++++++++++++++++++- 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 29901272984..9eb608bcfd9 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -8,6 +8,7 @@ ########################################################################## """A blueprint module implementing the sqleditor frame.""" +import codecs import os import pickle import re @@ -2192,8 +2193,29 @@ def start_query_download_tool(trans_id): else: output_encoding = 'utf-8' add_bom = False - is_utf = output_encoding.lower().replace('-', '').replace( - '_', '').startswith('utf') + # Validate the (free-text, user-configurable) encoding up front so + # an invalid codec returns a clean 400 here, rather than raising a + # LookupError mid-stream after the 200 Response has been returned. + try: + codecs.lookup(output_encoding) + except LookupError: + return make_json_response( + status=400, + success=0, + errormsg=gettext( + "Unknown output encoding '{0}'." + ).format(output_encoding) + ) + + normalized_encoding = output_encoding.lower().replace( + '-', '').replace('_', '') + is_utf = normalized_encoding.startswith('utf') + # The 'utf-16' and 'utf-32' codecs (without an explicit endianness + # suffix) emit their own BOM, so we must not hand-prepend one too; + # doing so would produce two BOMs and corrupt the output. The + # explicit-endian forms (utf-16-le/-be, utf-32-le/-be) and utf-8 do + # not self-emit a BOM, so for those we keep writing it ourselves. + codec_self_emits_bom = normalized_encoding in ('utf16', 'utf32') str_gen = gen(conn_obj, trans_obj, @@ -2208,7 +2230,9 @@ def encoded_gen(text_gen): for chunk in text_gen: if is_first_chunk: is_first_chunk = False - if add_bom and is_utf: + # Only hand-prepend a BOM when the codec does not emit + # one itself, otherwise we'd end up with two BOMs. + if add_bom and is_utf and not codec_self_emits_bom: chunk = '\ufeff' + chunk yield chunk.encode(output_encoding, errors='replace') diff --git a/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py b/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py index 17a1bd28f44..70550a60580 100644 --- a/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py +++ b/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py @@ -7,6 +7,7 @@ # This software is released under the PostgreSQL Licence # ########################################################################## +import codecs from unittest.mock import patch from pgadmin.utils.route import BaseTestGenerator @@ -276,6 +277,23 @@ class TestDownloadResultFormats(BaseTestGenerator): expected_content_type='text/csv', expected_extension='.csv') ), + ( + # utf-16 (without endianness) self-emits a BOM, so the result + # must contain exactly one BOM, not two (a hand-prepended one + # plus the codec's own). + 'Download CSV as utf-16 has exactly one BOM', + dict(data_format='csv', add_bom=True, encoding='utf-16', + expected_content_type='text/csv', + expected_extension='.csv') + ), + ( + # A bogus, non-existent codec must be rejected up front with a + # clean 400, rather than blowing up mid-stream after a 200. + 'Download CSV with an invalid output encoding returns 400', + dict(data_format='csv', add_bom=False, encoding='not-a-codec', + expected_status=400, expected_content_type=None, + expected_extension='.csv') + ), ] def setUp(self): @@ -327,6 +345,18 @@ def runTest(self): self.app.logger.disabled = False headers = dict(response.headers) + + # An invalid encoding must be rejected up front with a clean error + # status, before the streaming Response is constructed. + expected_status = getattr(self, 'expected_status', 200) + if expected_status != 200: + self.assertEqual(response.status_code, expected_status) + url = '/sqleditor/close/{0}'.format(self.trans_id) + response = self.tester.delete(url) + self.assertEqual(response.status_code, 200) + database_utils.disconnect_database(self, self._sid, self._did) + return + self.assertEqual(response.status_code, 200) self.assertIn(self.expected_content_type, headers['Content-Type']) self.assertIn('charset={0}'.format(self.encoding), @@ -334,8 +364,19 @@ def runTest(self): self.assertIn(filename, headers['Content-Disposition']) raw = response.data - if self.add_bom and self.encoding.lower().startswith('utf'): - self.assertTrue(raw.startswith(b'\xef\xbb\xbf')) + normalized = self.encoding.lower().replace('-', '').replace('_', '') + if self.add_bom and normalized.startswith('utf'): + # The output must carry exactly one BOM for the encoding, never + # two (which happened when a BOM was hand-prepended for codecs + # that already self-emit one, e.g. utf-16/utf-32). + bom = { + 'utf8': codecs.BOM_UTF8, + 'utf16': codecs.BOM_UTF16, + 'utf32': codecs.BOM_UTF32, + }[normalized] + self.assertTrue(raw.startswith(bom)) + # No second, redundant BOM immediately after the first. + self.assertFalse(raw[len(bom):].startswith(bom)) else: self.assertFalse(raw.startswith(b'\xef\xbb\xbf')) From cb21494ca583e0bc6c7f37f8612e876a33e047ad Mon Sep 17 00:00:00 2001 From: Dave Page Date: Mon, 17 Aug 2026 13:46:53 +0100 Subject: [PATCH 3/5] Make the JSON and XML output parseable whatever the data is Four things were wrong with the new formats, and I checked each against the running server rather than taking them on trust, which is worth saying because two of the four turned out differently from the review. XML was genuinely broken: xml.sax.saxutils.escape() handles the three markup characters and passes everything else through, but XML 1.0 forbids most C0 control characters outright, and they cannot be escaped as character references either. A text column holding chr(1), which PostgreSQL is perfectly happy to store, produced a document that ElementTree rejects with "not well-formed (invalid token)". Those characters are now replaced with U+FFFD, in element text and in the column name attributes alike. NaN and Infinity were genuinely broken too: json.dumps writes them as bare tokens, which Python reads back but most other parsers refuse, so they now become the strings PostgreSQL itself uses. Containers are walked on the way out, since a float8[] or a json column can hold them nested. The bytea case reported in review does not arise on this path: the query tool registers a loader that reports the placeholder "binary data" for bytea, as the grid and the existing CSV export both show, so nothing here ever sees a memoryview. The isinstance handling is still there, cheap insurance if that loader is ever changed, but it is not fixing a live bug. Content-Disposition needed the quoting the review asked for. A name with a space was truncated by the client, so it is quoted now, and where a name cannot be encoded as latin-1 the real name is sent as RFC 5987 filename* rather than being discarded in favour of a hardcoded download.csv, whose extension was wrong for JSON and XML anyway. One further problem the review did not reach: both new formats applied the "Replace null values with" preference, so every NULL arrived as the string "NULL". That preference exists because CSV cannot distinguish an empty field from a NULL; JSON has null and the XML here has null="true", so both now report NULLs natively and the preference applies to CSV only. Tests cover a row containing a forbidden control character, a bytea value, NaN, both infinities and a NULL, asserting that the output parses with a strict parser in each format, plus the two filename cases. --- web/pgadmin/tools/sqleditor/__init__.py | 31 +++-- .../tests/test_download_csv_query_tool.py | 121 +++++++++++++++++- .../utils/driver/psycopg3/connection.py | 93 +++++++++++--- 3 files changed, 211 insertions(+), 34 deletions(-) diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 9eb608bcfd9..864a55bec44 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -13,7 +13,7 @@ import pickle import re import secrets -from urllib.parse import unquote +from urllib.parse import quote as url_quote, unquote from threading import Lock from io import BytesIO import threading @@ -2259,18 +2259,27 @@ def encoded_gen(text_gen): filename = data['filename'] if data.get('filename', '') != "" else \ '{0}.{1}'.format(int(time.time()), extn) - # We will try to encode report file name with latin-1 - # If it fails then we will fallback to default ascii file name - # werkzeug only supports latin-1 encoding supported values + # Werkzeug will only put latin-1 in a header, so a name it cannot + # encode needs an ASCII stand-in. RFC 6266 lets us send the real name + # alongside it as filename*, so rather than losing the name entirely + # we offer both and let the client prefer the latter. The fallback + # follows the chosen format rather than always claiming to be a CSV. + ascii_filename = filename try: - tmp_file_name = filename - tmp_file_name.encode('latin-1', 'strict') + filename.encode('latin-1', 'strict') except UnicodeEncodeError: - filename = "download.csv" - - r.headers[ - "Content-Disposition" - ] = "attachment;filename={0}".format(filename) + ascii_filename = 'download.{0}'.format(extn) + + # RFC 6266 requires the quoted form for anything with a space or a + # separator character in it, which a user supplied name can easily + # have. + disposition = 'attachment; filename="{0}"'.format( + ascii_filename.replace('\\', '\\\\').replace('"', '\\"')) + if ascii_filename != filename: + disposition += "; filename*=UTF-8''{0}".format( + url_quote(filename, safe='')) + + r.headers["Content-Disposition"] = disposition return r except (ConnectionLost, SSHTunnelConnectionLost): diff --git a/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py b/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py index 70550a60580..ca33e8e8ba5 100644 --- a/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py +++ b/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py @@ -9,6 +9,8 @@ ########################################################################## import codecs from unittest.mock import patch +from urllib.parse import quote as url_quote +from xml.etree import ElementTree from pgadmin.utils.route import BaseTestGenerator from pgadmin.browser.server_groups.servers.databases.tests import utils as \ @@ -243,6 +245,19 @@ def tearDown(self): test_utils.drop_database(main_conn, self._db_name) +# A control character that XML 1.0 does not allow at all (not even as a +# character reference), a bytea value, the two non-finite floats that JSON +# has no syntax for, and a NULL. +AWKWARD_SQL = ( + 'SELECT E\'ctl\\x01char\' as "Ctl", ' + '\'\\x48656c6c6f\'::bytea as "Bytes", ' + '\'NaN\'::float8 as "NotANumber", ' + '\'Infinity\'::float8 as "Inf", ' + '\'-Infinity\'::float8 as "NegInf", ' + 'NULL::text as "Nothing"' +) + + class TestDownloadResultFormats(BaseTestGenerator): """ Validates downloading query results as JSON and XML, the UTF BOM option @@ -294,8 +309,49 @@ class TestDownloadResultFormats(BaseTestGenerator): expected_status=400, expected_content_type=None, expected_extension='.csv') ), + ( + # RFC 6266 requires the quoted form once the name contains a + # space, or the client sees a truncated filename. + 'Download with a filename containing spaces', + dict(data_format='csv', add_bom=False, encoding='utf-8', + expected_content_type='text/csv', + expected_extension='.csv', + filename_override='my query results.csv') + ), + ( + # A name werkzeug cannot put in a latin-1 header still has to + # reach the client, via the RFC 5987 filename* form, rather than + # being thrown away. + 'Download with a filename outside latin-1', + dict(data_format='csv', add_bom=False, encoding='utf-8', + expected_content_type='text/csv', + expected_extension='.csv', + filename_override='ohms-\u03a9.csv') + ), + ( + # Data that the naive serialisers get wrong: a control character + # that XML 1.0 forbids outright, a bytea column, the non-finite + # floats that are not valid JSON, and a NULL. + 'Download awkward data as JSON stays valid JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json', sql=AWKWARD_SQL, + awkward_data=True) + ), + ( + 'Download awkward data as XML stays well formed', + dict(data_format='xml', add_bom=False, encoding='utf-8', + expected_content_type='application/xml', + expected_extension='.xml', sql=AWKWARD_SQL, + awkward_data=True) + ), ] + # Set per scenario; the scenarios above override these as needed. + sql = None + awkward_data = False + filename_override = None + def setUp(self): self._db_name = 'download_results_fmt_' + str( secrets.choice(range(10000, 65535))) @@ -311,6 +367,46 @@ def initiate_sql_query_tool(self, trans_id, sql_query): return async_poll(tester=self.tester, poll_url='/sqleditor/poll/{0}'.format(trans_id)) + def _assert_awkward_data(self, body): + """The output must be parseable, whatever the data contained. + + A strict parser is the point here: XML 1.0 forbids most control + characters outright, and NaN/Infinity are not JSON tokens, so an + exporter that passes them straight through produces a file the + user's next tool refuses to open. + """ + if self.data_format == 'json': + def reject_constant(constant): + # json.loads accepts NaN and Infinity by default even though + # they are not JSON; most other parsers do not, so treat them + # as the failure they are. + raise AssertionError( + '{0} is not a JSON token'.format(constant)) + + parsed = json.loads(body, parse_constant=reject_constant) + self.assertEqual(len(parsed), 1) + row = parsed[0] + # A NULL must survive as JSON null rather than a string. + self.assertIsNone(row['Nothing']) + # NaN and Infinity have to arrive as something a parser will + # accept, i.e. not as bare NaN/Infinity tokens. + self.assertEqual(row['NotANumber'], 'NaN') + self.assertEqual(row['Inf'], 'Infinity') + self.assertEqual(row['NegInf'], '-Infinity') + # bytea is deliberately reported as a placeholder rather than its + # contents, as it is in the grid and in CSV output, but it must + # never leak a Python repr such as ''. + self.assertNotIn('memory at', str(row['Bytes'])) + return + + root = ElementTree.fromstring(body) + self.assertEqual(root.tag, 'data_output') + columns = {c.get('name'): c for c in root.find('row')} + self.assertEqual(columns['Nothing'].get('null'), 'true') + self.assertNotIn('memory at', columns['Bytes'].text or '') + # The control character must not have been passed through verbatim. + self.assertNotIn('\x01', body) + def runTest(self): db_con = database_utils.connect_database(self, test_utils.SERVER_GROUP, @@ -327,17 +423,19 @@ def runTest(self): })) self.assertEqual(response.status_code, 200) - self.initiate_sql_query_tool(self.trans_id, self.SQL) + sql = self.sql or self.SQL + self.initiate_sql_query_tool(self.trans_id, sql) url = self.DOWNLOAD_URL.format(self.trans_id) self.app.logger.disabled = True - filename = 'test{0}'.format(self.expected_extension) + filename = self.filename_override or \ + 'test{0}'.format(self.expected_extension) with patch('pgadmin.tools.sqleditor.blueprint.' 'csv_add_bom.get', return_value=self.add_bom), \ patch('pgadmin.tools.sqleditor.blueprint.' 'csv_output_encoding.get', return_value=self.encoding): response = self.tester.post(url, data={ - "query": self.SQL, + "query": sql, "filename": filename, "format": self.data_format, "query_commited": True, @@ -361,7 +459,18 @@ def runTest(self): self.assertIn(self.expected_content_type, headers['Content-Type']) self.assertIn('charset={0}'.format(self.encoding), headers['Content-Type']) - self.assertIn(filename, headers['Content-Disposition']) + disposition = headers['Content-Disposition'] + try: + filename.encode('latin-1', 'strict') + except UnicodeEncodeError: + # The stand-in must follow the format, and the real name must + # still be there in percent-encoded form. + self.assertIn('filename="download{0}"'.format( + self.expected_extension), disposition) + self.assertIn("filename*=UTF-8''", disposition) + self.assertIn(url_quote(filename, safe=''), disposition) + else: + self.assertIn('filename="{0}"'.format(filename), disposition) raw = response.data normalized = self.encoding.lower().replace('-', '').replace('_', '') @@ -382,7 +491,9 @@ def runTest(self): body = raw.decode(self.encoding) - if self.data_format == 'json': + if self.awkward_data: + self._assert_awkward_data(body) + elif self.data_format == 'json': parsed = json.loads(body) self.assertIsInstance(parsed, list) self.assertEqual(parsed[0]['A'], 1) diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index deb2867ea8e..175c7735078 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -18,7 +18,9 @@ import datetime import asyncio import json +import re from collections import deque +from math import isfinite, isnan from xml.sax.saxutils import escape as xml_escape, quoteattr as xml_quoteattr import psycopg from flask import g, current_app @@ -63,38 +65,97 @@ def _json_default(value): return str(value) -def _generate_json(cur, records, results, header, replace_nulls_with, - handle_null_values): +# XML 1.0 permits tab, newline, carriage return and nothing else below U+0020, +# and forbids the surrogate range and U+FFFE/U+FFFF. Those characters cannot +# even be written as character references, so a parser rejects the whole +# document: a text column legally holding chr(1) would otherwise produce a +# file nothing can open. +_XML_ILLEGAL_CHARS = re.compile( + '[^\u0009\u000a\u000d\u0020-\ud7ff\ue000-\ufffd' + '\U00010000-\U0010ffff]' +) + +# Substituted for anything XML cannot carry. +_XML_REPLACEMENT = '\ufffd' + + +def _to_text(value): + """Render a value as text for the structured output formats.""" + if isinstance(value, (memoryview, bytes, bytearray)): + # Match the hex form PostgreSQL itself uses for bytea, rather than + # letting str() produce something like ''. + return '\\x' + bytes(value).hex() + if isinstance(value, float) and not isfinite(value): + return 'NaN' if isnan(value) else \ + ('Infinity' if value > 0 else '-Infinity') + return str(value) + + +def _xml_text(value): + """Escape a value for XML, dropping characters XML cannot represent.""" + return xml_escape( + _XML_ILLEGAL_CHARS.sub(_XML_REPLACEMENT, _to_text(value))) + + +def _xml_attr(value): + """Quote an attribute value for XML, with the same sanitising.""" + return xml_quoteattr(_XML_ILLEGAL_CHARS.sub(_XML_REPLACEMENT, str(value))) + + +def _json_safe(value): + """Convert a value into something json can encode, and validly. + + NaN and Infinity are not JSON tokens: json.dumps emits them bare by + default, which Python itself will read back but most other parsers + reject, so they become the strings PostgreSQL uses for them. bytea is + rendered in the same hex form as elsewhere. Containers are walked + because a float8[] or a json column can hold either case nested. + """ + if isinstance(value, (memoryview, bytes, bytearray)): + return _to_text(value) + if isinstance(value, float) and not isfinite(value): + return _to_text(value) + if isinstance(value, dict): + return {key: _json_safe(val) for key, val in value.items()} + if isinstance(value, (list, tuple)): + return [_json_safe(val) for val in value] + return value + + +def _generate_json(cur, records, results): """Stream the result set as a JSON array of row objects. The first batch of rows (``results``) has already been fetched by the caller; subsequent batches are pulled with ``fetchmany(records)``. + + The 'Replace null values with' preference is deliberately not applied: + it exists because CSV has no way to distinguish an empty field from a + NULL, whereas JSON has null, and substituting the placeholder string + would turn every NULL into ordinary text. """ yield '[' is_first_row = True while results: - if replace_nulls_with is not None: - results = handle_null_values(results, replace_nulls_with) for row in results: - row_json = json.dumps(dict(row), default=_json_default) + row_json = json.dumps( + {key: _json_safe(value) for key, value in dict(row).items()}, + default=_json_default, allow_nan=False) yield row_json if is_first_row else ',' + row_json is_first_row = False results = cur.fetchmany(records) yield ']' -def _generate_xml(cur, records, results, header, replace_nulls_with, - handle_null_values): +def _generate_xml(cur, records, results, header): """Stream the result set as XML. Column names are emitted as escaped ``name`` attributes (rather than element names) so that column names which are not valid XML element - names are handled safely. + names are handled safely. As with JSON, NULLs are reported natively, + via null="true", rather than through the CSV placeholder preference. """ yield '\n' while results: - if replace_nulls_with is not None: - results = handle_null_values(results, replace_nulls_with) for row in results: row_io = [''] for column in header: @@ -102,10 +163,10 @@ def _generate_xml(cur, records, results, header, replace_nulls_with, if value is None: row_io.append( ''.format( - xml_quoteattr(column))) + _xml_attr(column))) else: row_io.append('{1}'.format( - xml_quoteattr(column), xml_escape(str(value)))) + _xml_attr(column), _xml_text(value))) row_io.append('') yield ''.join(row_io) results = cur.fetchmany(records) @@ -1010,13 +1071,9 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", json_columns.append(column_name) if data_format == 'json': - yield from _generate_json(cur, records, results, header, - replace_nulls_with, - handle_null_values) + yield from _generate_json(cur, records, results) elif data_format == 'xml': - yield from _generate_xml(cur, records, results, header, - replace_nulls_with, - handle_null_values) + yield from _generate_xml(cur, records, results, header) else: res_io = StringIO() From f1da04a2a83f7c0df650a268cfe44f954ccea094 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Thu, 20 Aug 2026 09:16:34 +0100 Subject: [PATCH 4/5] Emit direct single-value output and fix empty-result content type for JSON/XML export. Addresses the remaining CodeRabbit findings on #10062: - A genuine single-row, single-column result is now written as the bare value (a JSON scalar, or an XML document with no / wrapper), per #3205, instead of always being wrapped in a one-element array or a element. - An empty (zero-row) result now yields an empty JSON array or an empty XML document for those formats, rather than the CSV-era plain-text "did not return any data" message under an application/json or application/xml content type. - Added regression coverage for both cases (including the NULL single-value shape), and extended the Latin-1 output-encoding test to use a character Latin-1 cannot represent (previously ASCII-only, so it could not have caught silent character loss), asserting the existing errors='replace' contract is preserved end to end. --- .../tests/test_download_csv_query_tool.py | 110 +++++++++++++++++- .../utils/driver/psycopg3/connection.py | 50 +++++++- 2 files changed, 156 insertions(+), 4 deletions(-) diff --git a/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py b/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py index ca33e8e8ba5..b2bf87f0ad9 100644 --- a/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py +++ b/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py @@ -287,10 +287,17 @@ class TestDownloadResultFormats(BaseTestGenerator): expected_extension='.csv') ), ( + # '€' (Euro sign) cannot be represented in Latin-1. The + # exporter's errors='replace' contract must survive the whole + # request/response round trip: an ASCII-only fixture would let a + # regression that silently drops or mis-encodes the character + # pass unnoticed. 'Download CSV with a non-UTF output encoding', dict(data_format='csv', add_bom=True, encoding='latin-1', expected_content_type='text/csv', - expected_extension='.csv') + expected_extension='.csv', + sql='SELECT 1 as "A", 2 as "B", \'€\' as "C"', + non_latin1_char='€') ), ( # utf-16 (without endianness) self-emits a BOM, so the result @@ -345,12 +352,69 @@ class TestDownloadResultFormats(BaseTestGenerator): expected_extension='.xml', sql=AWKWARD_SQL, awkward_data=True) ), + ( + # A genuine single-row, single-column result must be written as + # the bare value, not wrapped in the usual array/row structure, + # per #3205. + 'Download a single-value result as JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json', + sql='SELECT 42 as "Value"', single_value=True) + ), + ( + 'Download a single-value result as XML', + dict(data_format='xml', add_bom=False, encoding='utf-8', + expected_content_type='application/xml', + expected_extension='.xml', + sql='SELECT 42 as "Value"', single_value=True) + ), + ( + # A single-row, single-column NULL is still the direct-value + # shape, i.e. a bare JSON null / an empty element with + # null="true", not a row containing one null column. + 'Download a single-value NULL result as JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json', + sql='SELECT NULL::text as "Value"', single_value=True, + single_value_is_null=True) + ), + ( + 'Download a single-value NULL result as XML', + dict(data_format='xml', add_bom=False, encoding='utf-8', + expected_content_type='application/xml', + expected_extension='.xml', + sql='SELECT NULL::text as "Value"', single_value=True, + single_value_is_null=True) + ), + ( + # Zero rows must still come back as a (empty) document of the + # requested format, not the CSV-era plain-text message under an + # application/json or application/xml content type. + 'Download an empty result as JSON stays valid JSON', + dict(data_format='json', add_bom=False, encoding='utf-8', + expected_content_type='application/json', + expected_extension='.json', + sql='SELECT 1 as "A" WHERE false', empty_result=True) + ), + ( + 'Download an empty result as XML stays well formed', + dict(data_format='xml', add_bom=False, encoding='utf-8', + expected_content_type='application/xml', + expected_extension='.xml', + sql='SELECT 1 as "A" WHERE false', empty_result=True) + ), ] # Set per scenario; the scenarios above override these as needed. sql = None awkward_data = False + single_value = False + single_value_is_null = False + empty_result = False filename_override = None + non_latin1_char = None def setUp(self): self._db_name = 'download_results_fmt_' + str( @@ -407,6 +471,40 @@ def reject_constant(constant): # The control character must not have been passed through verbatim. self.assertNotIn('\x01', body) + def _assert_single_value(self, body): + """A genuine single-row, single-column result must be the bare + value, per #3205, not a one-element array / one-row document. + """ + if self.data_format == 'json': + parsed = json.loads(body) + if self.single_value_is_null: + self.assertIsNone(parsed) + else: + self.assertEqual(parsed, 42) + return + + root = ElementTree.fromstring(body) + self.assertEqual(root.tag, 'data_output') + # No row/column wrapper, and no column name anywhere in sight. + self.assertIsNone(root.find('row')) + self.assertIsNone(root.find('column')) + if self.single_value_is_null: + self.assertEqual(root.get('null'), 'true') + else: + self.assertEqual(root.text, '42') + + def _assert_empty_result(self, body): + """Zero rows must still come back as an (empty) document of the + requested format, not the CSV-era plain-text message. + """ + if self.data_format == 'json': + self.assertEqual(json.loads(body), []) + return + + root = ElementTree.fromstring(body) + self.assertEqual(root.tag, 'data_output') + self.assertEqual(list(root), []) + def runTest(self): db_con = database_utils.connect_database(self, test_utils.SERVER_GROUP, @@ -493,6 +591,10 @@ def runTest(self): if self.awkward_data: self._assert_awkward_data(body) + elif self.single_value: + self._assert_single_value(body) + elif self.empty_result: + self._assert_empty_result(body) elif self.data_format == 'json': parsed = json.loads(body) self.assertIsInstance(parsed, list) @@ -506,6 +608,12 @@ def runTest(self): self.assertIn('', body) else: self.assertIn('"A","B","C"', body) + if self.non_latin1_char: + # errors='replace' must turn the character Latin-1 cannot + # encode into the codec's standard replacement rather than + # silently dropping it or corrupting the row. + self.assertNotIn(self.non_latin1_char, body) + self.assertIn('?', body) url = '/sqleditor/close/{0}'.format(self.trans_id) response = self.tester.delete(url) diff --git a/web/pgadmin/utils/driver/psycopg3/connection.py b/web/pgadmin/utils/driver/psycopg3/connection.py index 175c7735078..a770e1c6955 100644 --- a/web/pgadmin/utils/driver/psycopg3/connection.py +++ b/web/pgadmin/utils/driver/psycopg3/connection.py @@ -173,6 +173,23 @@ def _generate_xml(cur, records, results, header): yield '' +def _generate_single_value(data_format, value): + """Render a genuine single-row, single-column result directly, per + issue #3205: no array wrapper for JSON, no / wrapper for + XML, just the value itself. NULL is reported the same way it is + elsewhere: JSON null, or an empty element with null="true". + """ + if data_format == 'json': + return json.dumps( + _json_safe(value), default=_json_default, allow_nan=False) + + if value is None: + return ('\n' + '') + return ('\n' + '{0}'.format(_xml_text(value))) + + # Register global type caster which will be applicable to all connections. register_global_typecasters() configure_driver_encodings(encodings) @@ -1056,9 +1073,6 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", # Make sure numeric values will be fetched without quoting register_numeric_typecasters(cur) results = cur.fetchmany(records) - if not results: - yield gettext('The query executed did not return any data.') - return header = [] json_columns = [] @@ -1070,6 +1084,36 @@ def gen(conn_obj, trans_obj, quote='strings', quote_char="'", if c.to_dict()['type_code'] in ALL_JSON_TYPES: json_columns.append(column_name) + if not results: + # An empty result must still come back in the requested + # format: JSON/XML consumers expect a (empty) document of + # that type, not the CSV-era plain-text message under an + # application/json or application/xml content type. + if data_format == 'json': + yield '[]' + elif data_format == 'xml': + yield ('\n' + '') + else: + yield gettext( + 'The query executed did not return any data.') + return + + if data_format in ('json', 'xml') and len(header) == 1 and \ + len(results) == 1: + # A genuine single-row, single-column result is written as + # the bare value, without the usual array/row wrapper, per + # #3205. Confirm there really is only one row before + # committing to that shape: a batch boundary can make the + # first fetchmany() return exactly one row even though more + # follow. + more = cur.fetchmany(records) + if not more: + yield _generate_single_value( + data_format, results[0].get(header[0])) + return + results = results + more + if data_format == 'json': yield from _generate_json(cur, records, results) elif data_format == 'xml': From 15ec50e900528565bc5854dbf000f03142ac2e35 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Tue, 1 Sep 2026 12:13:13 +0100 Subject: [PATCH 5/5] Emit one BOM per file rather than one per chunk, and clean up in tearDown The exporter fetches ten rows at a time and encoded each chunk with its own chunk.encode() call. The 'utf-16' and 'utf-32' codecs emit their BOM on every such call, so any export large enough to span more than one chunk carried a BOM at the head of every chunk rather than once at the head of the file. The existing utf-16 scenario could not catch it: its fixture is a single row, which never spans chunks. Encoding now goes through an incremental encoder, which emits the BOM once and then keeps going, with a final flush for anything the codec is holding. The utf-16 scenario is joined by a 25 row one, and the assertion no longer just checks that a second BOM does not immediately follow the first: it counts them across the whole payload. The fixture data is ASCII, so the BOM byte sequence cannot occur in the encoded rows and the count is exact. Closing the transaction and disconnecting have moved into tearDown, so a failed assertion no longer skips them and leaves the connection open against a database that is about to be dropped. --- web/pgadmin/tools/sqleditor/__init__.py | 14 +++++++++- .../tests/test_download_csv_query_tool.py | 27 ++++++++++++++++--- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py index 864a55bec44..24b2ffb34f9 100644 --- a/web/pgadmin/tools/sqleditor/__init__.py +++ b/web/pgadmin/tools/sqleditor/__init__.py @@ -2225,6 +2225,14 @@ def start_query_download_tool(trans_id): replace_nulls_with=blueprint.replace_nulls_with.get(), data_format=data_format) + # Encode incrementally rather than a chunk at a time. The codecs + # that emit their own BOM do so on every encode() call, so encoding + # each chunk on its own would put a BOM at the head of every chunk + # rather than once at the head of the file; an incremental encoder + # emits it once and then keeps going. + encoder = codecs.getincrementalencoder(output_encoding)( + errors='replace') + def encoded_gen(text_gen): is_first_chunk = True for chunk in text_gen: @@ -2234,7 +2242,11 @@ def encoded_gen(text_gen): # one itself, otherwise we'd end up with two BOMs. if add_bom and is_utf and not codec_self_emits_bom: chunk = '\ufeff' + chunk - yield chunk.encode(output_encoding, errors='replace') + yield encoder.encode(chunk) + + trailing = encoder.encode('', True) + if trailing: + yield trailing if data_format == 'json': base_mimetype = 'application/json' diff --git a/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py b/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py index b2bf87f0ad9..c5edf46a995 100644 --- a/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py +++ b/web/pgadmin/tools/sqleditor/tests/test_download_csv_query_tool.py @@ -308,6 +308,18 @@ class TestDownloadResultFormats(BaseTestGenerator): expected_content_type='text/csv', expected_extension='.csv') ), + ( + # The exporter fetches ten rows at a time, and the codecs that + # self-emit a BOM do so on every encode() call, so anything + # spanning more than one chunk is where a BOM per chunk would + # show up. A single row cannot catch that. + 'Download multi-chunk CSV as utf-16 has exactly one BOM', + dict(data_format='csv', add_bom=True, encoding='utf-16', + expected_content_type='text/csv', + expected_extension='.csv', + sql='SELECT g as "A", g * 2 as "B", \'x\' as "C" ' + 'FROM generate_series(1, 25) g') + ), ( # A bogus, non-existent codec must be rejected up front with a # clean 400, rather than blowing up mid-stream after a 200. @@ -584,6 +596,12 @@ def runTest(self): self.assertTrue(raw.startswith(bom)) # No second, redundant BOM immediately after the first. self.assertFalse(raw[len(bom):].startswith(bom)) + # And none further in either: the exporter encodes in chunks, + # and a codec that emits its own BOM emits one per encode() + # call, so a payload spanning several chunks is where a stray + # BOM would otherwise appear. The fixture data is ASCII, so the + # BOM byte sequence cannot occur in the encoded rows. + self.assertEqual(raw.count(bom), 1) else: self.assertFalse(raw.startswith(b'\xef\xbb\xbf')) @@ -615,12 +633,15 @@ def runTest(self): self.assertNotIn(self.non_latin1_char, body) self.assertIn('?', body) + def tearDown(self): + # Closing the transaction and disconnecting belong here rather than + # at the end of runTest: a failed assertion would otherwise skip + # them and leave the connection open against a database that is + # about to be dropped. url = '/sqleditor/close/{0}'.format(self.trans_id) - response = self.tester.delete(url) - self.assertEqual(response.status_code, 200) + self.tester.delete(url) database_utils.disconnect_database(self, self._sid, self._did) - def tearDown(self): main_conn = test_utils.get_db_connection( self.server['db'], self.server['username'],