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

Send scroll_id inside a json rather that text #1325

Merged
merged 1 commit into from Jun 13, 2017

Conversation

Projects
None yet
4 participants
@nomoa
Contributor

nomoa commented Jun 9, 2017

It's likely to cause issues with elastic 5.3 as Content-Type handling becomes
more and more strict.

eg.:
curl -H"Content-Type: application/json" -XGET localhost:9200/_search/scroll?scroll=1m -d 'DXF...'
now fails with Failed to parse request body on 5.3.x.
Elastic 5.1 was lenient and accepted this body and content-type.
Since we default Content-Type to application/json this is likely cause issues
in Elastica as well.
Using an array instead of plain string should force the use of a JSON body which is coherent
with the content-type we set.

@bgaleotti

This comment has been minimized.

Show comment
Hide comment
@bgaleotti

bgaleotti commented Jun 10, 2017

@ruflin

This comment has been minimized.

Show comment
Hide comment
@ruflin

ruflin Jun 13, 2017

Owner

@nomoa LGTM. Could you add a changelog entry?

Owner

ruflin commented Jun 13, 2017

@nomoa LGTM. Could you add a changelog entry?

@ruflin

ruflin approved these changes Jun 13, 2017

Send scroll_id inside a json rather that text
It's likely to cause issues with elastic 5.3 as Content-Type handling becomes
more and more strict.

eg.:
curl -H"Content-Type: application/json"  -XGET localhost:9200/_search/scroll?scroll=1m -d 'DXF...'
now fails with Failed to parse request body on 5.3.x.
Elastic 5.1 was lenient and accepted this body.
Since we default Content-Type to application/json this is likely cause issues
in Elastica as well.
Using an array instead of plain string should force the use of a JSON body which is coherent
with the content-type we set.
@nomoa

This comment has been minimized.

Show comment
Hide comment
@nomoa

nomoa Jun 13, 2017

Contributor

@ruflin done

Contributor

nomoa commented Jun 13, 2017

@ruflin done

@ruflin ruflin merged commit e40ca53 into ruflin:master Jun 13, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ruflin

This comment has been minimized.

Show comment
Hide comment
@ruflin

ruflin Jun 13, 2017

Owner

@nomoa Merged. Thanks.

Owner

ruflin commented Jun 13, 2017

@nomoa Merged. Thanks.

@etudor

This comment has been minimized.

Show comment
Hide comment
@etudor

etudor Jul 20, 2017

Had the same problem with: ruflin/elastica 5.2.1 and elasticsearch 5.4.0

{
  "error": {
    "root_cause": [
      {
        "type": "illegal_argument_exception",
        "reason": "Failed to parse request body"
      }
    ],
    "type": "illegal_argument_exception",
    "reason": "Failed to parse request body",
    "caused_by": {
      "type": "json_parse_exception",
      "reason": "Unrecognized token 'DXF1ZXJ5QW5kRmV0Y2gBAAAAAAAHGKoWY3h2eGxBcHZUUGlINHRDa21USy0xUQ': was expecting ('true', 'false' or 'null')\n at [Source: org.elasticsearch.transport.netty4.ByteBufStreamInput@6d20e23; line: 1, column: 64]"
    }
  },
  "status": 400
}

Updating ruflin/elastica to dev-master solved the problem.

etudor commented Jul 20, 2017

Had the same problem with: ruflin/elastica 5.2.1 and elasticsearch 5.4.0

{
  "error": {
    "root_cause": [
      {
        "type": "illegal_argument_exception",
        "reason": "Failed to parse request body"
      }
    ],
    "type": "illegal_argument_exception",
    "reason": "Failed to parse request body",
    "caused_by": {
      "type": "json_parse_exception",
      "reason": "Unrecognized token 'DXF1ZXJ5QW5kRmV0Y2gBAAAAAAAHGKoWY3h2eGxBcHZUUGlINHRDa21USy0xUQ': was expecting ('true', 'false' or 'null')\n at [Source: org.elasticsearch.transport.netty4.ByteBufStreamInput@6d20e23; line: 1, column: 64]"
    }
  },
  "status": 400
}

Updating ruflin/elastica to dev-master solved the problem.

@ruflin

This comment has been minimized.

Show comment
Hide comment
@ruflin

ruflin Jul 27, 2017

Owner

The 5.3 release went out yesterday which should include this bugfix.

Owner

ruflin commented Jul 27, 2017

The 5.3 release went out yesterday which should include this bugfix.

qubot pushed a commit to artefactual/atom that referenced this pull request Sep 29, 2017

Update Elasticsearch deps to v5.3.0, refs #11534
Updates Elastica and elasticsearch-php to v5.3.0 to fix issue with scroll API.

Issue detailed here: ruflin/Elastica#1325

qubot pushed a commit to artefactual/atom that referenced this pull request Sep 29, 2017

Update Elasticsearch deps to v5.3.0, refs #11567
Updates Elastica and elasticsearch-php to v5.3.0 to fix issue with scroll API.

Issue detailed here: ruflin/Elastica#1325

qubot pushed a commit to artefactual/atom that referenced this pull request Oct 13, 2017

Update Elasticsearch deps to v5.3.0, refs #11567
Updates Elastica and elasticsearch-php to v5.3.0 to fix issue with scroll API.

Issue detailed here: ruflin/Elastica#1325

g011um added a commit to g011um/atom that referenced this pull request Oct 26, 2017

Update Elasticsearch deps to v5.3.0, refs #11567
Updates Elastica and elasticsearch-php to v5.3.0 to fix issue with scroll API.

Issue detailed here: ruflin/Elastica#1325
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment