Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Support legacy Elastic _bulk API clients which use Content-Type: application/json #602

Open
awesense-paul opened this issue Dec 12, 2024 · 4 comments

Comments

@awesense-paul
Copy link

Background
  • The Elastic _bulk API is effectively multiple JSON objects concatenated one after the other; separated by newlines.
  • Older Elasticsearch clients (e.g. rsyslog, others) of the _bulk API may specify a legacy/incorrect Content-Type header of application/json (this implies a single JSON document). The newer -- correct -- Content-Type is "Content-Type: application/x-ndjson", but since this is a newer standard, the old clients never implemented using it.
  • qryn allows only application/x-ndjson, and rejects requests which use application/json.
Change Request

If qryn permitted "Content-Type: application/json", this would allow the older clients to be supported without changes. (Even though this is not strictly correct in the current time.) Perhaps it could be a setting whether to permit it or not?

@lmangani
Copy link
Collaborator

Thanks for raising the issue and for the matrix discussion motivating it. Overall this should be relatively simple but might have side effects so it needs a bit of research and testing. If you want to hack this in and test locally, you can simply change or extend this code block

@awesense-paul
Copy link
Author

awesense-paul commented Dec 12, 2024

This was the change that I found worked for me:

awesense-paul@9ad456e

(the mentioned code block actually had the same body in both the if and else. I think there's a better fix that actually makes the if block check for application/json and avoid doing req.body.split, and where the else block -- now handling x-ndjson -- uses _rawBody, but tbh I don't really understand the codebase+fastify well enough to know what to do.)

@awesense-paul
Copy link
Author

I think there's a better fix

(or not, since the later handling does a JSON.parse() on each element in streams anyway)

@lmangani
Copy link
Collaborator

@awesense-paul please send in a PR and we'll adjust and merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants