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

Hex numbers in YAML requests are treated as strings #66555

Open
andrewkroh opened this issue Dec 17, 2020 · 5 comments · Fixed by #72833 or #72995
Open

Hex numbers in YAML requests are treated as strings #66555

andrewkroh opened this issue Dec 17, 2020 · 5 comments · Fixed by #72833 or #72995
Labels
>bug :Core/Infra/REST API REST infrastructure and utilities help wanted adoptme Team:Core/Infra Meta label for core/infra team

Comments

@andrewkroh
Copy link
Member

Elasticsearch version (bin/elasticsearch --version):

{
  "number": "7.11.0-SNAPSHOT",
  "build_flavor": "default",
  "build_type": "docker",
  "build_hash": "00e58bef3ac3f0bd7ab68b90e38e11cac05fb49e",
  "build_date": "2020-12-15T14:29:43.737519Z",
  "build_snapshot": true,
  "lucene_version": "8.7.0",
  "minimum_wire_compatibility_version": "6.8.0",
  "minimum_index_compatibility_version": "6.0.0-beta1"
}

Plugins installed: []

JVM version (java -version):

{
  "version": "15.0.1",
  "vm_name": "OpenJDK 64-Bit Server VM",
  "vm_version": "15.0.1+9",
  "vm_vendor": "AdoptOpenJDK",
  "bundled_jdk": true,
  "using_bundled_jdk": true,
  "count": 1
}

OS version (uname -a if on a Unix-like system): Our official Docker container based on CentOS Linux 8

Description of the problem including expected versus actual behavior:

When Elasticsearch unmarshals YAML scalar values in hexadecimal format it interprets the value as a string, but it should be a number. For example 0x1 becomes "0x01" when it should a 1. This causes problems when you expected the data type to be a number (like when reading the param value within painless).

Reference: https://yaml.org/spec/1.2/spec.html#id2766934

Relates: elastic/kibana#85486

Steps to reproduce:

  1. Create an ingest pipeline using Content-Type: application/yaml. Include a hexidecimal number.
curl \
  -u elastic:changeme \
  -H 'Content-Type: application/yaml' \
  -X PUT \
  "localhost:9200/_ingest/pipeline/yaml_test" \
  -d'---

description: YAML unmarshal bug demo
processors:
  - script:
      lang: painless
      params:
        number_base10: 1
        number_base16: 0x1
        string: hello
      source: >
        params.forEach((k, v) -> ctx[k] = v);'
  1. Read the pipeline.
curl -u elastic:changeme -H 'Content-Type: application/yaml' -X GET "localhost:9200/_ingest/pipeline/yaml_test"
---
yaml_test:
  description: "YAML unmarshal bug demo"
  processors:
  - script:
      lang: "painless"
      params:
        number_base10: 1
        number_base16: "0x1"
        string: "hello"
      source: "params.forEach((k, v) -> ctx[k] = v);"
$ curl -u elastic:changeme -X GET "localhost:9200/_ingest/pipeline/yaml_test?pretty"
{
  "yaml_test" : {
    "description" : "YAML unmarshal bug demo",
    "processors" : [
      {
        "script" : {
          "lang" : "painless",
          "params" : {
            "number_base10" : 1,
            "number_base16" : "0x1",
            "string" : "hello"
          },
          "source" : "params.forEach((k, v) -> ctx[k] = v);"
        }
      }
    ]
  }
}

Note that number_base16 is "0x1". It should a 1.

@andrewkroh andrewkroh added >bug needs:triage Requires assignment of a team area label labels Dec 17, 2020
@andrewkroh
Copy link
Member Author

I was curious so I started looking upstream and found FasterXML/jackson-dataformats-text#233 which appears to have been fixed in 2.12.

@jtibshirani jtibshirani added :Core/Infra/REST API REST infrastructure and utilities and removed needs:triage Requires assignment of a team area label labels Dec 18, 2020
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Dec 18, 2020
@elasticmachine
Copy link
Collaborator

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

@rjernst
Copy link
Member

rjernst commented Dec 18, 2020

Thanks for reporting and the investigation upstream @andrewkroh. It looks like 2.12 was just released a few weeks ago. I'm going to mark this as help wanted so anyone can pick it up if they feel so empowered. In theory, simply bumping our Jackson version should enable support.

@jamesrowe08
Copy link

Bumped version of jackson and tested this. Please have a look @elasticmachine

rjernst added a commit to rjernst/elasticsearch that referenced this issue May 6, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes elastic#66555
closes elastic#67214
rjernst added a commit that referenced this issue May 7, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes #66555
closes #67214
rjernst added a commit to rjernst/elasticsearch that referenced this issue May 12, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes elastic#66555
closes elastic#67214
rjernst added a commit that referenced this issue May 27, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes #66555
closes #67214

Co-authored-by: Francisco Fernández Castaño <[email protected]>
rjernst added a commit to rjernst/elasticsearch that referenced this issue May 27, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes elastic#66555
closes elastic#67214

Co-authored-by: Francisco Fernández Castaño <[email protected]>
rjernst added a commit that referenced this issue May 27, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes #66555
closes #67214

backport #72995
backport #73011

Co-authored-by: Francisco Fernández Castaño <[email protected]>
Co-authored-by: Mark Vieira <[email protected]>
@rjernst
Copy link
Member

rjernst commented Jun 7, 2021

#72995 has to be reverted, so the upgrade of Jackson fixing this hex issue needs to be reopened.

@rjernst rjernst reopened this Jun 7, 2021
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Nov 1, 2021
This grabs a new version of jackson and snakeyaml to get some bug fixes.
It also updates azure to stay compatible with the new jackson.

Closes elastic#66555
Closes elastic#67214
Closes elastic#80142
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Nov 1, 2021
This grabs a new version of jackson and snakeyaml to get some bug fixes.
It also updates azure to stay compatible with the new jackson.

Closes elastic#66555
Closes elastic#67214
Closes elastic#80142
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/REST API REST infrastructure and utilities help wanted adoptme Team:Core/Infra Meta label for core/infra team
Projects
None yet
5 participants