Skip to content

Conversation

@johannesduesing
Copy link

@johannesduesing johannesduesing commented Jun 18, 2020

Reason for this PR
As reported in #34, the error messages returned by the webapi in cases of invalid queries / invalid query limits / errors during database interaction have all been 500 Internal Server Errors, and even exposed some application internals.
This implies that regular users may not understand the cause for a query failure, and malicious users may learn something about private data.

Changes in this PR
Introduced distinct HttpResonses on different errors:

  • If the execution of an ElasticSearch query fails due to an unknown reason, a 500 Internal Server Error will be returned. The StatusDescription will contain a JSON object holding a msg ("Database Communication Failed") and a query field, the latter holding the original query that triggered the error.
  • If the maximum query limit (10000) is exceeded by the client, then a 400 Bad Request is returned. The StatusDescription also holds the aforementioned JSON object, with the msg stating that the limit has been exceeded, including the value of the limit.
  • If the query could not be parsed, a 400 Bad Request is returned. The StatusDescription also holds the aforementioned JSON object, with the msg explaining that there was a syntax error in the query, including the line and column of the error.
  • If the query Syntax was valid but contained unknown field names, a 400 Bad Request is returned. The StatusDescription also holds the aforementioned JSON object. The msg states that there were unknown field names, and an additional attribute invalid_fields holds an array of all invalid field names.
  • All other (unexpected) errors will results in a 500 Internal Server Error

Additional Context
A upcoming PR for the delphi-cli will introduce support for the 400 Bad Request responses, which will then print the error messages in order to explain search errors to the user. EDIT: PR is at delphi-hub/delphi-cli#46

Issues

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@johannesduesing
Copy link
Author

@bhermann Can you look at this and tell me if you're fine with the changes i introduced?

Copy link
Member

@bhermann bhermann left a comment

Choose a reason for hiding this comment

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

That look good to me!

@bhermann bhermann merged commit a4766bc into develop Jul 7, 2020
@bhermann bhermann deleted the bugfix/errormessages branch July 7, 2020 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants