Skip to content

Commit cf0772e

Browse files
committed
Emit CSV error when no header row has been written
If a csv page errored before its first data row, columns was empty and write_record produced an empty record, dropping the error message. Emit a single-field record in that case. Adds a regression test.
1 parent 0b24f2f commit cf0772e

3 files changed

Lines changed: 47 additions & 9 deletions

File tree

src/render.rs

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -713,15 +713,22 @@ impl CsvBodyRenderer {
713713

714714
pub async fn handle_error(&mut self, error: &anyhow::Error) -> anyhow::Result<()> {
715715
let err_str = error_message(error, self.is_prod);
716-
self.writer
717-
.write_record(
718-
self.columns
719-
.iter()
720-
.enumerate()
721-
.map(|(i, _)| if i == 0 { err_str.as_str() } else { "" })
722-
.collect::<Vec<_>>(),
723-
)
724-
.await?;
716+
if self.columns.is_empty() {
717+
// The error happened before any header row was written, so there are
718+
// no columns to align with. Emit a single-field record so the error
719+
// is still reported instead of an empty record.
720+
self.writer.write_record([err_str.as_str()]).await?;
721+
} else {
722+
self.writer
723+
.write_record(
724+
self.columns
725+
.iter()
726+
.enumerate()
727+
.map(|(i, _)| if i == 0 { err_str.as_str() } else { "" })
728+
.collect::<Vec<_>>(),
729+
)
730+
.await?;
731+
}
725732
Ok(())
726733
}
727734

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
select 'csv' as component;
2+
-- Error before any data row: the CSV header is never written, so columns is
3+
-- empty when handle_error runs.
4+
select * from definitely_missing_table_xyz;

tests/data_formats/mod.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,33 @@ async fn test_prod_csv_error_does_not_leak_sql() {
312312
assert_no_sql_leak(&body, "csv error");
313313
}
314314

315+
/// A CSV page can hit an error before its first data row (so no header has been
316+
/// written and `columns` is empty). The generic error message must still be
317+
/// emitted instead of an empty record.
318+
#[actix_web::test]
319+
async fn test_prod_csv_error_before_any_row_still_reports() {
320+
let app_data = make_prod_app_data().await;
321+
if matches!(
322+
app_data.db.info.database_type,
323+
sqlpage::webserver::database::SupportedDatabase::Oracle
324+
) {
325+
return;
326+
}
327+
let req = TestRequest::get()
328+
.uri("/tests/data_formats/csv_error_no_rows.sql")
329+
.insert_header((header::ACCEPT, "text/csv"))
330+
.app_data(app_data)
331+
.to_srv_request();
332+
let resp = main_handler(req).await.expect("handler should not fail");
333+
let body = test::read_body(resp).await;
334+
let body = String::from_utf8(body.to_vec()).unwrap();
335+
assert!(
336+
body.to_lowercase().contains("administrator"),
337+
"csv error before the first row must still emit the generic error message: {body:?}"
338+
);
339+
assert_no_sql_leak(&body, "csv error before any row");
340+
}
341+
315342
/// An author may only intend a page to be served as HTML, but a client can
316343
/// request it with `Accept: application/json` and pick the JSON renderer.
317344
/// In production that path must not leak SQL text either.

0 commit comments

Comments
 (0)