Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Added support to ipv4 and ipv6 on URL validation #67

Merged
merged 2 commits into from
Jul 9, 2019
Merged

Added support to ipv4 and ipv6 on URL validation #67

merged 2 commits into from
Jul 9, 2019

Conversation

ricardobessadacosta
Copy link
Contributor

Related to issue: #14

Description:
Replaced the old regex to accept IPv4 and IPv6 on Webhook URL based on RFC 3986.

  host        = IP-literal / IPv4address / reg-name
  IP-literal = "[" ( IPv6address / IPvFuture  ) "]"
  IPv4address = dec-octet "." dec-octet "." dec-octet "." dec-octet

  dec-octet   = DIGIT                 ; 0-9
              / %x31-39 DIGIT         ; 10-99
              / "1" 2DIGIT            ; 100-199
              / "2" %x30-34 DIGIT     ; 200-249
              / "25" %x30-35          ; 250-255
  IPv6address =                            6( h16 ":" ) ls32
              /                       "::" 5( h16 ":" ) ls32
              / [               h16 ] "::" 4( h16 ":" ) ls32
              / [ *1( h16 ":" ) h16 ] "::" 3( h16 ":" ) ls32
              / [ *2( h16 ":" ) h16 ] "::" 2( h16 ":" ) ls32
              / [ *3( h16 ":" ) h16 ] "::"    h16 ":"   ls32
              / [ *4( h16 ":" ) h16 ] "::"              ls32
              / [ *5( h16 ":" ) h16 ] "::"              h16
              / [ *6( h16 ":" ) h16 ] "::"

  ls32        = ( h16 ":" h16 ) / IPv4address
              ; least-significant 32 bits of address

  h16         = 1*4HEXDIG
              ; 16 bits of address represented in hexadecimal

Tried to let the regex more readable and easy to understand. Suggestions would be appreciated.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

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

Hi @ricardobessa1

Thank you for your contribution. Could you add few Unit Test cases for this change ?

@ricardobessadacosta
Copy link
Contributor Author

Hello @mihirsoni ,

I have added some Unit Test cases.

Copy link
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

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

Thank for changes ! Looks good to me.

Copy link
Contributor

@dbbaughe dbbaughe left a comment

Choose a reason for hiding this comment

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

Thanks!

@dbbaughe dbbaughe merged commit 7920a33 into opendistro-for-elasticsearch:master Jul 9, 2019
@Cyb3rSn0rlax
Copy link

still having this issue with plugin 1.7.0 kibana version 7.6.1:
image

@Lyr
Copy link

Lyr commented Jun 10, 2020

still having this issue with plugin 1.7.0 kibana version 7.6.1:
image

I guess this is caused by the version of your browser, I've got this same issue on my side and I've fixed with another browser.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants