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

[Security] Include an empty json object in an json array when FLS filters out all fields #30709

Conversation

@martijnvg
Copy link
Member

martijnvg commented May 18, 2018

Prior to this change an json array element with no fields would be omitted from json array.
Nested inner hits source filtering relies on the fact that the json array element numbering
remains untouched and this causes AOOB exceptions on the ES side during the fetch phase
without this change.

Closes #30624

@elasticmachine

This comment has been minimized.

Copy link
Collaborator

elasticmachine commented May 18, 2018

@jimczi
jimczi approved these changes May 18, 2018
Copy link
Member

jimczi left a comment

LGTM

...c/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/FieldSubsetReader.java Outdated
@@ -195,6 +195,8 @@ public Fields getTermVectors(int docID) throws IOException {
Map<String, Object> filteredValue = filter((Map<String, ?>)value, includeAutomaton, state);
if (filteredValue.isEmpty() == false) {
filtered.add(filteredValue);
} else {
filtered.add(new HashMap<>());

This comment has been minimized.

Copy link
@jimczi

jimczi May 18, 2018

Member

Maybe use the empty filteredValue directly ?

This comment has been minimized.

Copy link
@martijnvg

martijnvg May 18, 2018

Author Member

Good point. I've updated the PR.

martijnvg added 2 commits May 18, 2018
…ters out all fields

Prior to this change an json array element with no fields would be omitted from json array.
Nested inner hits source filtering relies on the fact that the json array element numbering
remains untouched and this causes AOOB exceptions in the ES side during the fetch phase
without this change.

Closes #30624
@martijnvg martijnvg force-pushed the martijnvg:security_fls_nested_inner_hits_source_filtering branch to b959164 May 22, 2018
@martijnvg martijnvg merged commit 25959ed into elastic:master May 22, 2018
3 checks passed
3 checks passed
CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci Build finished.
Details
elasticsearch-ci/packaging-sample Build finished.
Details
martijnvg added a commit that referenced this pull request May 22, 2018
…ters out all fields (#30709)

Prior to this change an json array element with no fields would be omitted from json array.
Nested inner hits source filtering relies on the fact that the json array element numbering
remains untouched and this causes AOOB exceptions in the ES side during the fetch phase
without this change.

Closes #30624
martijnvg added a commit that referenced this pull request May 22, 2018
…ters out all fields (#30709)

Prior to this change an json array element with no fields would be omitted from json array.
Nested inner hits source filtering relies on the fact that the json array element numbering
remains untouched and this causes AOOB exceptions in the ES side during the fetch phase
without this change.

Closes #30624
dnhatn added a commit that referenced this pull request May 22, 2018
* master:
  QA: Add xpack tests to rolling upgrade (#30795)
  Modify state of VerifyRepositoryResponse for bwc (#30762)
  Reduce CLI scripts to one-liners on Windows (#30772)
  Simplify number of shards setting (#30783)
  Replace Request#setHeaders with addHeader (#30588)
  [TEST] remove endless wait in RestClientTests (#30776)
  [Docs] Fix script-fields snippet execution (#30693)
  Upgrade to Lucene-7.4.0-snapshot-cc2ee23050 (#30778)
  [DOCS] Add SAML configuration information (#30548)
  [DOCS] Remove X-Pack references from SQL CLI (#30694)
  Make http pipelining support mandatory (#30695)
  [Docs] Fix typo in circuit breaker docs (#29659)
  [Feature] Adding a char_group tokenizer (#24186)
  [Docs] Fix broken cross link in documentation
  Test: wait for netty threads in a JUnit ClassRule (#30763)
  Increase the maximum number of filters that may be in the cache. (#30655)
  [Security] Include an empty json object in an json array when FLS filters out all fields (#30709)
  [TEST] Wait for CS to be fully applied in testDeleteCreateInOneBulk
  Add more yaml tests for get alias API (#29513)
  Ignore empty completion input (#30713)
  [DOCS] fixed incorrect default
  [ML] Filter undefined job groups from update calendar actions (#30757)
  Fix docs failure on language analyzers (#30722)
  [Docs] Fix inconsistencies in snapshot/restore doc (#30480)
  Enable installing plugins from snapshots.elastic.co (#30765)
  Remove fedora 26, add 28 (#30683)
  Accept Gradle build scan agreement (#30645)
  Remove logging from elasticsearch-nio jar (#30761)
  Add Delete Repository High Level REST API (#30666)
dnhatn added a commit that referenced this pull request May 24, 2018
* 6.x:
  [DOCS] Fixes typos in security settings
  Add support for indexed shape routing in geo_shape query (#30760)
  [DOCS] Splits auditing.asciidoc into smaller files
  Painless: Types Section Clean Up (#30283)
  [test] java tests for archive packaging (#30734)
  Deprecate http.pipelining setting (#30786)
  [DOCS] Fix more edit URLs in Stack Overview (#30704)
  Use correct cluster state version for node fault detection (#30810)
  [DOCS] Fixes broken link for native realm
  [DOCS] Clarified audit.index.client.hosts (#30797)
  Change serialization version of doc-value fields.
  Add a `format` option to `docvalue_fields`. (#29639)
  [TEST] Don't expect acks when isolating nodes
  Fixes UpdateSettingsRequestStreamableTests mutate bug
  Revert "Add more yaml tests for get alias API (#29513)"
  Revert "Mutes MachineLearningTests.testNoAttributes_givenSameAndMlEnabled"
  Only allow x-pack metadata if all nodes are ready (#30743)
  Mutes MachineLearningTests.testNoAttributes_givenSameAndMlEnabled
  Use original settings on full-cluster restart (#30780)
  Only ack cluster state updates successfully applied on all nodes (#30672)
  Replace Request#setHeaders with addHeader (#30588)
  [TEST] remove endless wait in RestClientTests (#30776)
  QA: Add xpack tests to rolling upgrade (#30795)
  Add support for search templates to the high-level REST client. (#30473)
  Reduce CLI scripts to one-liners on Windows (#30772)
  Fold RestGetAllSettingsAction in RestGetSettingsAction (#30561)
  Add more yaml tests for get alias API (#29513)
  [Docs] Fix script-fields snippet execution (#30693)
  Convert FieldCapabilitiesResponse to a ToXContentObject. (#30182)
  Remove assert statements from field caps documentation. (#30601)
  Fix a bug in FieldCapabilitiesRequest#equals and hashCode. (#30181)
  Add support for field capabilities to the high-level REST client. (#29664)
  [DOCS] Add SAML configuration information (#30548)
  [DOCS] Remove X-Pack references from SQL CLI (#30694)
  [Docs] Fix typo in circuit breaker docs (#29659)
  [Feature] Adding a char_group tokenizer (#24186)
  Increase the maximum number of filters that may be in the cache. (#30655)
  [Docs] Fix broken cross link in documentation
  Test: wait for netty threads in a JUnit ClassRule (#30763)
  [Security] Include an empty json object in an json array when FLS filters out all fields (#30709)
  [DOCS] fixed incorrect default
  [TEST] Wait for CS to be fully applied in testDeleteCreateInOneBulk
  Enable installing plugins from snapshots.elastic.co (#30765)
  Ignore empty completion input (#30713)
  Fix docs failure on language analyzers (#30722)
  [Docs] Fix inconsistencies in snapshot/restore doc (#30480)
  Add Delete Repository High Level REST API (#30666)
  Reduce CLI scripts to one-liners (#30759)
@jpountz jpountz added v6.3.0 and removed v6.3.1 labels Jun 13, 2018
@colings86 colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.