Skip to content

ETT-1352 imgsrv/download/pdf should not output stack traces with a ba…#195

Merged
moseshll merged 2 commits intomainfrom
ETT-1352_bad_seq
Mar 18, 2026
Merged

ETT-1352 imgsrv/download/pdf should not output stack traces with a ba…#195
moseshll merged 2 commits intomainfrom
ETT-1352_bad_seq

Conversation

@moseshll
Copy link
Contributor

@moseshll moseshll commented Mar 13, 2026

…d seq

  • Remove imgsrv seq parameters that do not conform to \d+, \d+-\d+, or \d+,\d+...
  • Add superficial playwright test against example which previously would produce a 500 error

@carylwyatt if you would critique the Playwright side, that was a semi-sloppy afterthought and probably could be done better.

…d seq

- Remove imgsrv `seq` parameters that do not conform to \d+, \d+-\d+, or \d+,\d+...
- Add superficial playwright test against example which previously would produce a 500 error
@moseshll moseshll requested review from aelkiss and carylwyatt March 16, 2026 13:55
Copy link
Member

@aelkiss aelkiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All makes sense to me.

@carylwyatt
Copy link
Member

That playwright test is small but fine! I tested current behavior vs this change (with just the bad seq parameter but also with the rest of the URL that pt would likely create for download on that volume /cgi/imgsrv/download/pdf?id=test.pd_open&attachment=1&tracker=D1&seq=mashed_potatoes(includes tracker and attachment params) and that also returned 200, so this looks good 👍

@moseshll moseshll merged commit 6d4b4e7 into main Mar 18, 2026
3 of 4 checks passed
@moseshll moseshll deleted the ETT-1352_bad_seq branch March 18, 2026 18:01
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.

3 participants