Skip to content
This repository has been archived by the owner on Dec 22, 2022. It is now read-only.

API server crashes with OutOfMemoryError if invalid query is used #16

Closed
tdddblog opened this issue Apr 15, 2021 · 6 comments
Closed

API server crashes with OutOfMemoryError if invalid query is used #16

tdddblog opened this issue Apr 15, 2021 · 6 comments
Assignees
Labels
B12.0 bug Something isn't working c.api s.high

Comments

@tdddblog
Copy link
Contributor

tdddblog commented Apr 15, 2021

🐛 Describe the bug

API server crashes with OutOfMemoryError if invalid query is used

📜 To Reproduce

Steps to reproduce the behavior:

  1. Go to Swagger UI (http://localhost:8080)
  2. Click on 'collections' link.
  3. Enter invalid query title=Kaguya in 'q' parameter
  4. Hit 'Try it out!' button.
  5. UI hangs for 20-30 seconds, then returns "Internal Server Error" 500.
  6. Server logs show java.lang.OutOfMemeory error.

🕵️ Expected behavior

  1. API should return error message that = sign is not supported in queries.
  2. Server should not crash with out of memory error.

📚 Version of Software Used

0.1.0

🖥 System Info

  • OS: Windows 10
  • Browser: Firefox
  • Version: 87.0

** 🦄 Applicable requirements**

@tloubrieu-jpl
Copy link
Member

@al-niessner has rewritten the query parsing for ticket NASA-PDS/registry-api#457. This bug need to be tested against the rewritten parsing.

@al-niessner
Copy link
Contributor

al-niessner commented Apr 20, 2021

@tloubrieu-jpl @jordanpadams

With updates on pds-api-54 and related PR, no longer have OutOfMemoryError but still have a 500 because the grammar exception is not being mapped into an HTTP error code.

@al-niessner
Copy link
Contributor

@tloubrieu-jpl @jordanpadams

Not any more. Parse errors are returned as HTTP errors. ANTLR interface fixed so that it does not run out of memory anymore. All work done detached for other reasons on other branches. How do we close this ticket?

@jordanpadams
Copy link
Member

@tloubrieu-jpl how do we want to test this out? should we have @al-niessner add a test case to prove this?

@tloubrieu-jpl
Copy link
Member

tloubrieu-jpl commented May 19, 2021

@jordanpadams I will review Al's tickets in the afternoon. I will update my postman test suite with these cases. @tdddblog should be able to use it in the integration test later.

@tloubrieu-jpl
Copy link
Member

This bug has been corrected by syntax lexer re-engineering for ticket NASA-PDS/registry-api#457

The request now return 400 bad request

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
B12.0 bug Something isn't working c.api s.high
Projects
None yet
Development

No branches or pull requests

4 participants