Skip to content

Conversation

@pgomulka
Copy link
Contributor

@pgomulka pgomulka commented Jan 19, 2021

Stored scripts can have content_type option set, however when empty they default to XContentType.JSON#mediaType(). Commit 5e74f79 has changed this in master (ES8) method to return application/json;charset=utf-8 (previously application/json; charset=UTF-8)
This means that when upgrading ES from version 7 to 8 stored script will fail when being used as the encoder is being matched with string equality (map key)

private static final Map<String, Supplier<Encoder>> ENCODERS = Map.of(
JSON_MIME_TYPE_WITH_CHARSET, JsonEscapeEncoder::new,
JSON_MIME_TYPE, JsonEscapeEncoder::new,
PLAIN_TEXT_MIME_TYPE, DefaultEncoder::new,
X_WWW_FORM_URLENCODED_MIME_TYPE, UrlEncoder::new);

This commit address this by adding back (in addition) the old application/json; charset=UTF-8 into the encoders map.

closes #66986

@pgomulka pgomulka added the WIP label Jan 19, 2021
@pgomulka pgomulka self-assigned this Jan 19, 2021
@pgomulka pgomulka changed the title WIP init and debug script check WIP Make scripted search templates work with new mediaType from XContentType.JSON Jan 19, 2021
@pgomulka pgomulka changed the title WIP Make scripted search templates work with new mediaType from XContentType.JSON Make scripted search templates work with new mediaType from XContentType.JSON Jan 19, 2021
@pgomulka pgomulka added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v8.0.0 and removed WIP labels Jan 19, 2021
@pgomulka pgomulka marked this pull request as ready for review January 19, 2021 19:25
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Jan 19, 2021
@elasticmachine
Copy link
Collaborator

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

@pgomulka
Copy link
Contributor Author

I removed the Script.java related changes to a separate PR

Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for working on this!

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM

@pgomulka pgomulka merged commit c2c50d5 into elastic:master Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >non-issue Team:Core/Infra Meta label for core/infra team v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test failing due to wrong content_type - 10_basic/Verify that we can still find things with the template

5 participants