Skip to content

fix: call validate_import_csv_rows in parse_export_format - #831

Closed
PRAteek-singHWY wants to merge 3 commits into
OWASP:mainfrom
PRAteek-singHWY:fix/bug-hunt-01
Closed

fix: call validate_import_csv_rows in parse_export_format#831
PRAteek-singHWY wants to merge 3 commits into
OWASP:mainfrom
PRAteek-singHWY:fix/bug-hunt-01

Conversation

@PRAteek-singHWY

Copy link
Copy Markdown
Contributor

Summary

Fixes a runtime regression in CSV import parsing.

parse_export_format() currently calls validate_export_csv_rows(...), but the validator defined in spreadsheet_parsers.py is validate_import_csv_rows(...).
This causes:

NameError: name 'validate_export_csv_rows' is not defined

Context

This validation flow was already introduced during the MyOpenCRE work, where CSV import validation logic was moved into spreadsheet_parsers.py via validate_import_csv_rows (see discussion in #584 comment and related work in #682 / #683).

This PR is a focused follow-up to restore that intended path on latest main.

Change

  • Updated one line in application/utils/spreadsheet_parsers.py:
    • validate_export_csv_rows(lfile) -> validate_import_csv_rows(lfile)

Validation

  • Ran:
    • ./venv/bin/python -m pytest application/tests/spreadsheet_parsers_test.py -q
  • Result:
    • 2 passed

Scope

  • Minimal, single-line regression fix
  • No behavior changes beyond restoring the existing import validation function call

@PRAteek-singHWY

PRAteek-singHWY commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

Hi @northdpole @Pa04rth
This is a one-line regression fix, so I opened a direct PR instead of raising a separate issue to avoid extra triage overhead.

Context: I previously worked on the CSV import validation path during MyOpenCRE (validate_import_csv_rows), so I recognized this call-site mismatch quickly. Related discussion: #584 (comment), #682, #683.

AyeshaaRafaqat added a commit to AyeshaaRafaqat/OpenCRE that referenced this pull request Aug 11, 2026
…e_import_csv_rows (OWASP#554)

OWASP#682/OWASP#683 already landed the intended shared validator,
spreadsheet_parsers.validate_import_csv_rows, but the live MyOpenCRE
import path (myopencre_parser -> export_format_parser.parse_export_format)
never called it, and this PR's own validate_cre_csv_rows duplicated the
same check instead of reusing it.

- Drop myopencre_parser.validate_cre_csv_rows; parse_rows_to_documents
  now calls spreadsheet_parsers.validate_import_csv_rows directly.
- Tighten validate_import_csv_rows's CRE-cell check to XXX-XXX|Name so
  every caller gets the stricter format, not just MyOpenCRE imports.
- Keep the web_main UTF-8 decode / triple-quote / csv.Error -> 400
  guards on /rest/v1/cre_csv_import unchanged.
- Move the CRE-cell format unit tests to spreadsheet_parsers_test.py,
  where the validator now lives; leave a thin wiring test in
  myopencre_parser_test.py proving parse_rows_to_documents delegates
  to the shared validator. web_main_test.py's HTTP 400 coverage is
  unchanged.

Verified locally: black, mypy (no new errors vs. main baseline),
targeted + full web_main_test.py suite (pre-existing unrelated Redis
failures on gap-analysis endpoints aside), and a live run against
/rest/v1/cre_csv_import with well-formed and malformed CSVs.

OWASP#831 (NameError: validate_export_csv_rows) is now obsolete; already
fixed on main by a different commit (89b1643).
AyeshaaRafaqat added a commit to AyeshaaRafaqat/OpenCRE that referenced this pull request Aug 11, 2026
…e_import_csv_rows (OWASP#554)

OWASP#682/OWASP#683 already landed the intended shared validator,
spreadsheet_parsers.validate_import_csv_rows, but the live MyOpenCRE
import path (myopencre_parser -> export_format_parser.parse_export_format)
never called it, and this PR's own validate_cre_csv_rows duplicated the
same check instead of reusing it.

- Drop myopencre_parser.validate_cre_csv_rows; parse_rows_to_documents
  now calls spreadsheet_parsers.validate_import_csv_rows directly.
- Tighten validate_import_csv_rows's CRE-cell check to XXX-XXX|Name so
  every caller gets the stricter format, not just MyOpenCRE imports.
- Keep the web_main UTF-8 decode / triple-quote / csv.Error -> 400
  guards on /rest/v1/cre_csv_import unchanged.
- Move the CRE-cell format unit tests to spreadsheet_parsers_test.py,
  where the validator now lives; leave a thin wiring test in
  myopencre_parser_test.py proving parse_rows_to_documents delegates
  to the shared validator. web_main_test.py's HTTP 400 coverage is
  unchanged.

Verified locally: black, mypy (no new errors vs. main baseline),
targeted + full web_main_test.py suite (pre-existing unrelated Redis
failures on gap-analysis endpoints aside), and a live run against
/rest/v1/cre_csv_import with well-formed and malformed CSVs.

OWASP#831 (NameError: validate_export_csv_rows) is now obsolete; already
fixed on main by a different commit (89b1643).
northdpole pushed a commit that referenced this pull request Aug 18, 2026
…e_import_csv_rows (#554)

#682/#683 already landed the intended shared validator,
spreadsheet_parsers.validate_import_csv_rows, but the live MyOpenCRE
import path (myopencre_parser -> export_format_parser.parse_export_format)
never called it, and this PR's own validate_cre_csv_rows duplicated the
same check instead of reusing it.

- Drop myopencre_parser.validate_cre_csv_rows; parse_rows_to_documents
  now calls spreadsheet_parsers.validate_import_csv_rows directly.
- Tighten validate_import_csv_rows's CRE-cell check to XXX-XXX|Name so
  every caller gets the stricter format, not just MyOpenCRE imports.
- Keep the web_main UTF-8 decode / triple-quote / csv.Error -> 400
  guards on /rest/v1/cre_csv_import unchanged.
- Move the CRE-cell format unit tests to spreadsheet_parsers_test.py,
  where the validator now lives; leave a thin wiring test in
  myopencre_parser_test.py proving parse_rows_to_documents delegates
  to the shared validator. web_main_test.py's HTTP 400 coverage is
  unchanged.

Verified locally: black, mypy (no new errors vs. main baseline),
targeted + full web_main_test.py suite (pre-existing unrelated Redis
failures on gap-analysis endpoints aside), and a live run against
/rest/v1/cre_csv_import with well-formed and malformed CSVs.

#831 (NameError: validate_export_csv_rows) is now obsolete; already
fixed on main by a different commit (89b1643).
@northdpole

Copy link
Copy Markdown
Collaborator

Closing — the branch has no diff (+0/−0). The fix appears already present on main or was addressed elsewhere.

@northdpole northdpole closed this Aug 19, 2026
Bornunique911 pushed a commit to Bornunique911/OpenCRE that referenced this pull request Aug 20, 2026
…e_import_csv_rows (OWASP#554)

OWASP#682/OWASP#683 already landed the intended shared validator,
spreadsheet_parsers.validate_import_csv_rows, but the live MyOpenCRE
import path (myopencre_parser -> export_format_parser.parse_export_format)
never called it, and this PR's own validate_cre_csv_rows duplicated the
same check instead of reusing it.

- Drop myopencre_parser.validate_cre_csv_rows; parse_rows_to_documents
  now calls spreadsheet_parsers.validate_import_csv_rows directly.
- Tighten validate_import_csv_rows's CRE-cell check to XXX-XXX|Name so
  every caller gets the stricter format, not just MyOpenCRE imports.
- Keep the web_main UTF-8 decode / triple-quote / csv.Error -> 400
  guards on /rest/v1/cre_csv_import unchanged.
- Move the CRE-cell format unit tests to spreadsheet_parsers_test.py,
  where the validator now lives; leave a thin wiring test in
  myopencre_parser_test.py proving parse_rows_to_documents delegates
  to the shared validator. web_main_test.py's HTTP 400 coverage is
  unchanged.

Verified locally: black, mypy (no new errors vs. main baseline),
targeted + full web_main_test.py suite (pre-existing unrelated Redis
failures on gap-analysis endpoints aside), and a live run against
/rest/v1/cre_csv_import with well-formed and malformed CSVs.

OWASP#831 (NameError: validate_export_csv_rows) is now obsolete; already
fixed on main by a different commit (89b1643).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants