Skip to content

Throw a 400 error for malformed parsing input when missing element end - #145777

Merged
carlosdelest merged 5 commits into
elastic:mainfrom
carlosdelest:bugfix/parse-exception-when-no-closed-element
Apr 13, 2026
Merged

Throw a 400 error for malformed parsing input when missing element end#145777
carlosdelest merged 5 commits into
elastic:mainfrom
carlosdelest:bugfix/parse-exception-when-no-closed-element

Conversation

@carlosdelest

@carlosdelest carlosdelest commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

We can see suppressed rest errors when an end element is missing from parsing.

Retrievers is one place where this can happen:

java.lang.IllegalStateException: parser for [retriever] did not end on END_OBJECT
	at org.elasticsearch.xcontent@9.4.0/org.elasticsearch.xcontent.ObjectParser.throwMustEndOn(ObjectParser.java:672)
	at org.elasticsearch.xcontent@9.4.0/org.elasticsearch.xcontent.ObjectParser.parseSub(ObjectParser.java:643)
	at org.elasticsearch.xcontent@9.4.0/org.elasticsearch.xcontent.ObjectParser.parse(ObjectParser.java:316)
	at org.elasticsearch.xcontent@9.4.0/org.elasticsearch.xcontent.ConstructingObjectParser.parse(ConstructingObjectParser.java:167)
	at org.elasticsearch.xcontent@9.4.0/org.elasticsearch.xcontent.ConstructingObjectParser.apply(ConstructingObjectParser.java:159)
	at org.elasticsearch.inference@9.4.0/org.elasticsearch.xpack.inference.rank.textsimilarity.TextSimilarityRankRetrieverBuilder.fromXContent(TextSimilarityRankRetrieverBuilder.java:118)
	at org.elasticsearch.inference@9.4.0/org.elasticsearch.xpack.inference.InferencePlugin.lambda$getRetrievers$27(InferencePlugin.java:840)
	at org.elasticsearch.server@9.4.0/org.elasticsearch.search.SearchModule.lambda$registerRetriever$24(SearchModule.java:1287)
	at org.elasticsearch.xcontent@9.4.0/org.elasticsearch.xcontent.NamedXContentRegistry.parseNamedObject(NamedXContentRegistry.java:149)

This PR changes ObjectParser to throw an XContentParseException in case we're missing an end element (object or array).

Related:

carlosdelest
When parsing misses an end element for arrays or objects, it throws a…
2d5adf6 
… parsing exception instead of a server IllegalStateException
@carlosdelest carlosdelest added >bug :Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team auto-backport Automatically create backport pull requests when merged Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch :Search Relevance/Search Catch all for Search Relevance v9.4.0 v9.2.9 v8.19.15 v9.3.4 labels Apr 7, 2026
@elasticsearchmachine

elasticsearchmachine commented Apr 7, 2026

Copy link
Copy Markdown
Collaborator

Hi @carlosdelest, I've created a changelog YAML for you.

@github-actions

github-actions Bot commented Apr 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🔍 Preview links for changed docs

⏳ Building and deploying preview... View progress

This comment will be updated with preview links when the build is complete.

@github-actions

github-actions Bot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

ℹ️ Important: Docs version tagging

👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version.

We use applies_to tags to mark version-specific features and changes.

Expand for a quick overview

When to use applies_to tags:

✅ At the page level to indicate which products/deployments the content applies to (mandatory)
✅ When features change state (e.g. preview, ga) in a specific version
✅ When availability differs across deployments and environments

What NOT to do:

❌ Don't remove or replace information that applies to an older version
❌ Don't add new information that applies to a specific version without an applies_to tag
❌ Don't forget that applies_to tags can be used at the page, section, and inline level

🤔 Need help?

elasticsearchmachine and others added 3 commits April 7, 2026 08:25
@carlosdelest
carlosdelest marked this pull request as ready for review April 7, 2026 12:26
@carlosdelest
carlosdelest requested a review from a team as a code owner April 7, 2026 12:26
@elasticsearchmachine

elasticsearchmachine commented Apr 7, 2026

Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@elasticsearchmachine

elasticsearchmachine commented Apr 7, 2026

Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@coderabbitai

coderabbitai Bot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 59df315e-9f66-4b0d-88fe-1fb5edd4ea6a

📥 Commits

Reviewing files that changed from the base of the PR and between 9d694b8 and 2f082bd.

📒 Files selected for processing (3)
  • docs/changelog/145777.yaml
  • libs/x-content/src/main/java/org/elasticsearch/xcontent/ObjectParser.java
  • libs/x-content/src/test/java/org/elasticsearch/xcontent/ObjectParserTests.java
📝 Walkthrough

Walkthrough

The change modifies how ObjectParser validates that nested parsers end at expected tokens. The parseSub method now passes the active XContentParser instance to throwMustEndOn, which previously received only field name and expected end token. The throwMustEndOn method's exception type changes from IllegalStateException to XContentParseException, which captures precise token location information. Tests are updated to expect the new exception type and use substring matching for error message validation.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • 🛠️ Update Documentation: Commit on current branch
  • 🛠️ Update Documentation: Create PR

Comment @coderabbitai help to get the list of available commands and usage tips.

davidkyle
davidkyle approved these changes Apr 13, 2026
View reviewed changes

@davidkyle davidkyle 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.

LGTM

For those interested XContentParseException is mapped to a 400 BAD REQUEST status code here

IllegalArgumentExceptions are also 400 status codes. IllegalStateException is the problem because it maps to a 500 status code. IllegalStateException also appears in AbstractObjectParser. I think we should remove that usage too but not necessarily in this PR

@carlosdelest
carlosdelest merged commit 18c88fe into elastic:main Apr 13, 2026
35 checks passed
This was referenced Apr 13, 2026
Merged
Merged
Merged
carlosdelest added a commit to carlosdelest/elasticsearch that referenced this pull request Apr 13, 2026
@carlosdelest
Throw a 400 error for malformed parsing input when missing element end (
5430607 
elastic#145777)
@elasticsearchmachine

elasticsearchmachine commented Apr 13, 2026

Copy link
Copy Markdown
Collaborator

💚 Backport successful

Status Branch Result
✅ 9.2
✅ 8.19
✅ 9.3

carlosdelest added a commit to carlosdelest/elasticsearch that referenced this pull request Apr 13, 2026
@carlosdelest
Throw a 400 error for malformed parsing input when missing element end (
0019765 
elastic#145777)
carlosdelest added a commit to carlosdelest/elasticsearch that referenced this pull request Apr 13, 2026
@carlosdelest
Throw a 400 error for malformed parsing input when missing element end (
670419e 
elastic#145777)
elasticsearchmachine pushed a commit that referenced this pull request Apr 13, 2026
@carlosdelest
Throw a 400 error for malformed parsing input when missing element end (
cd01699 
#145777) (#146158)
elasticsearchmachine pushed a commit that referenced this pull request Apr 13, 2026
@carlosdelest
Throw a 400 error for malformed parsing input when missing element end (
ea7680b 
#145777) (#146157)
elasticsearchmachine pushed a commit that referenced this pull request Apr 13, 2026
@carlosdelest
Throw a 400 error for malformed parsing input when missing element end (
e9154f0 
#145777) (#146159)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidkyle davidkyle davidkyle approved these changes

Assignees

No one assigned

Labels

auto-backport Automatically create backport pull requests when merged >bug :Core/Infra/Core Core issues without another label :Search Relevance/Search Catch all for Search Relevance Team:Core/Infra Meta label for core/infra team Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v8.19.15 v9.2.9 v9.3.4 v9.5.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@carlosdelest @elasticsearchmachine @davidkyle @brianseeders