Skip to content

Add test cases for bad requests - #3247

Merged
labkey-jeckels merged 5 commits into
developfrom
fb_invalid_request_400s
Oct 15, 2026
Merged

labkey-jeckels merged 5 commits into
developfrom
fb_invalid_request_400s

Conversation

@labkey-adam

Copy link
Copy Markdown
Contributor

Rationale

Verify that malformed requests return the expected status codes

Related Pull Requests

@labkey-tchad labkey-tchad left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't seem particular to webdav. Consider adding these to SendErrorTest (and perhaps rename it to something more generic, like ErrorResponseTest).

@labkey-adam

Copy link
Copy Markdown
Contributor Author

This doesn't seem particular to webdav. Consider adding these to SendErrorTest (and perhaps rename it to something more generic, like ErrorResponseTest).

Moved the test cases and renamed the target test

@labkey-adam

Copy link
Copy Markdown
Contributor Author

ErrorResponseTest is green on this branch: https://teamcity.labkey.org/buildConfiguration/bt3/4217682?buildTab=tests&name=Error

@labkey-jeckels

Copy link
Copy Markdown
Contributor

This doesn't seem particular to webdav. Consider adding these to SendErrorTest (and perhaps rename it to something more generic, like ErrorResponseTest).

Moved the test cases and renamed the target test

I added a couple more test cases for scenarios that came up in my review of Adam's updates. One was a false alarm, the other warranted a tweak to the platform PR.

labkey-jeckels added a commit to LabKey/platform that referenced this pull request Oct 15, 2026
…8160)

## Rationale
Malformed requests, mostly from vulnerability scanners, are logged as
500s and reported to mothership as server errors. This PR returns a 4xx
for them instead, and stops reporting them.

Tomcat 11 throws on every parameter read once a query string or form
body fails to parse. Rendering the standard error page reads parameters
too, so even a 404 for such a request became a 500.

Triage notes for these mothership reports:
https://claude.ai/artifact/CtxKZANKRqfhZxs7PHEqBr

## Related Pull Requests
- LabKey/testAutomation#3247

## Changes
- Requests whose parameters can't be parsed get a plain 4xx response
using Tomcat's status code, without rendering an error page. They are
never reported to mothership.
- Multipart bodies that can't be parsed get a 400 on standard actions,
instead of being rethrown as a server error.
- WebDAV reports to mothership only for 5xx responses, and returns 400
for unparseable query strings.
- Added a unit test for the invalid-parameter handling.

Tested against a local server with probe-style requests: invalid UTF-8
and empty-name parameters, a forwarded image request, and broken
multipart POSTs to actions and WebDAV. All returned 400 with no errors
logged, and normal pages were unaffected.

## Tasks
- [x] Claude Code Review
- [x] Manual Testing - probably not needed
- [x] Test Automation
- [x] Code Review @labkey-adam

---------

Co-authored-by: Adam Rauch <adam@labkey.com>
@labkey-jeckels
labkey-jeckels merged commit 7008d3c into develop Oct 15, 2026
6 checks passed
@labkey-jeckels
labkey-jeckels deleted the fb_invalid_request_400s branch October 15, 2026 04:08
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