Validation of labels of domain names according to RFC 1035 #1515
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
guicassolato
force-pushed
the
63-char-host-name-labels
branch
from
e26b12b
to
97daded
Compare
hallelujah
reviewed
Jan 3, 2020
app/validators/uri_validator.rb
Outdated
| @@ -26,6 +26,10 @@ def valid_scheme?(scheme) | |||
| end | |||
| end | |||
|
|
|||
| def valid_host?(host) | |||
| host.present? && (host.size <= 255) && !host.split('.').any? { |label| label.size > 63 } | |||
There was a problem hiding this comment.
I would separate it in another method
Suggested change
| host.present? && (host.size <= 255) && !host.split('.').any? { |label| label.size > 63 } | |
| host.present? && valid_domain_parts? |
def valid_domain_parts
host.split('.').map(&:size).all?(&63.method(:'>'))
end
…racters for the overall name and its labels - overall host name must be 255 characters long or less - labels must be 63 characters long or less Closes https://issues.redhat.com/browse/THREESCALE-2932
guicassolato
force-pushed
the
63-char-host-name-labels
branch
from
97daded
to
6ffe6ce
Compare
Codecov Report
@@ Coverage Diff @@
## master #1515 +/- ##
=========================================
- Coverage 93% 93% -0.01%
=========================================
Files 2514 2514
Lines 83469 83503 +34
=========================================
+ Hits 77630 77658 +28
- Misses 5839 5845 +6
Continue to review full report at Codecov.
|
Martouta
approved these changes
Jan 7, 2020
hallelujah
approved these changes
Jan 9, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to RFC 1035, domain host names must be 255 characters-long or less, and its labels must be 63 characters-long or less. Currently we do not validate domain names against this constraint which is causing OpenShift to reject non-compliant routes created by Zync.
One not-so-obvious case where this issue can cause us trouble is when creating a new product. The
system_nameof the product, chosen by the user, will end up being used to generate the proxy endpoints (depending on the sandbox proxy config). A sufficiently longsystem_namemay produce a domain name containing a lable that is longer than 63 characters. The route will then fail to be created or updated by Zync, requiring a manual intervention by the user to fix it. The creation of the product succeeds in this case though; yet, the endpoints cannot be used.This PR fixes this behavior by enforcing every domain name validated with https://github.com/3scale/porta/blob/bdc91b894eac16fbd81afd4f05198eb5cb8beee9/app/validators/uri_validator.rb to comply with the limits of characters specified by the RFC. Moreover, it makes proxy endpoints to be validated using this validator instead of using the old simple regex
porta/app/models/proxy.rb
Line 26 in bdc91b8
The PR also makes sure that validation errors on the proxy endpoints due to this new requirement are propagated to the service, specifically marking the
system_nameattribute. This is done with the support of https://github.com/3scale/porta/blob/bdc91b894eac16fbd81afd4f05198eb5cb8beee9/app/services/service_creator.rb.Closes THREESCALE-2932.