Skip to content

Disallow introducing illegal object mappings (double '..')#22891

Merged
dakrone merged 1 commit intoelastic:masterfrom
dakrone:disallow-double-dot
Feb 1, 2017
Merged

Disallow introducing illegal object mappings (double '..')#22891
dakrone merged 1 commit intoelastic:masterfrom
dakrone:disallow-double-dot

Conversation

@dakrone
Copy link
Copy Markdown
Member

@dakrone dakrone commented Jan 31, 2017

This disallows object mappings that would accidentally create something like
foo..bar, which is then unparsable for the bar field as it does not know
what its parent is.

Resolves #22794

@dakrone dakrone added :Search Foundations/Mapping Index mappings, including merging and defining field types >bug v5.2.1 v5.3.0 v6.0.0-alpha1 labels Jan 31, 2017
Copy link
Copy Markdown
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what about splitting first and then checking whether the array contains an empty string? I think that would also cover two other cases we are interested in validating: leading dots and trailing dots? (just thinking out loud)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you reduce the visibility of this method?

@dakrone
Copy link
Copy Markdown
Member Author

dakrone commented Jan 31, 2017

Thanks @jpountz, I pushed a commit that addresses your feedback!

Copy link
Copy Markdown
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

LGTM

@dakrone dakrone force-pushed the disallow-double-dot branch from a6c588d to d19f6af Compare January 31, 2017 22:26
@dakrone
Copy link
Copy Markdown
Member Author

dakrone commented Feb 1, 2017

retest this please

This disallows object mappings that would accidentally create something like
`foo..bar`, which is then unparsable for the `bar` field as it does not know
what its parent is.

Resolves elastic#22794
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Search Foundations/Mapping Index mappings, including merging and defining field types v5.1.3 v5.2.1 v5.3.0 v6.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants