[Freeipa-devel] [PATCH] 062-065 Ticket 1466 Validate AAAA records as valid IPv6 prior to sending to server

[Petr Vobornik]

On 01/04/2012 04:53 AM, Endi Sukma Dewata wrote:
> On 1/3/2012 3:06 AM, Petr Vobornik wrote:
>> 1) 62-Added-support-of-custom-field-validators
>> 2) 63-Added-validation-logic-to-multivalued-text-field - depends on 1
>> 3) 64-Added-client-side-validation-of-A-and-AAAA-DNS-recor - depends on 1
>> 4) 65-Fixed-IPv6-validation-special-case-single-colon - minor fix
>
> ACK and pushed to master. There are some minor issues but they can be
> fixed later since they don't cause regression:

New patch attached.
>
> 1. The IPv4 validator can take quads or integer, but it looks like
> according to RFC 1035 section 3.4.1 (http://tools.ietf.org/html/rfc1035)
> the A record should only take quads:
>
> A records cause no additional section processing. The RDATA section
> of an A line in a master file is an Internet address expressed as four
> decimal numbers separated by dots without any imbedded spaces (e.g.,
> "10.2.0.52" or "192.0.5.6")

CLI also accepts integer formatted addresses and it also works with 
bind. So I would not change it or I would change it on both sides.

>
> 2. The DNS record adder dialog doesn't validate the A/AAAA records. We
> might want to define some hidden fields, one field for each record type,
> then the field will only appear if you select the type from the
> drop-down list. This way each field can have its own validators. This
> solution is similar to permission target widget and will be the basis
> for ticket #2208.

Done. I'm dynamically changing only validators for the field. In #2208 I 
will probably drastically change it. My idea is prepare nice metadata 
(like record type - validator mappings and such) and generate adder and 
edit dialogs.

>
> 3. The host adder dialog doesn't validate the IP address.

Validation added. Also be aware of ticket: 
https://fedorahosted.org/freeipa/ticket/2229
>
> 4. The widget_value_changed() in IPA.multivalued_field is not needed
> because it's identical to the one in IPA.field.

Removed

> 5. Existing problem, in the DNS Resource Records page the delete
> operation doesn't work. I think this will be addressed in #2094.
>

Will fix in #2094

> 6. Existing problem, in the extract_child_value() in
> IPA.multivalued_text_widget if the value is an empty array the function
> will return the array itself instead of empty string.

Fixed
> 7. Existing problem, in net.js:330 the error message should be for
> leading instead of trailing zeros.
>
Fixed

Some thoughts when I was implementing #2:
I wanted to use only one field and abandon the overriding of 
create_add_command in record_adder_dialog (I don't like that hack).
I was thinking about two possible solutions:
  1) Override field's save method and use something like this: 
"record[type_got_from_other_widget] = values". Problem is that this 
method doesn't work with command construction in adder dialog and 
details facet - they rely on fields.name will be different from the type.
  2) Second approach was to dynamically change field's name. This seemed 
bad because the field can be saved in fields map under original name, 
not his current name. (Adding and removing from the map seemed even worse).

In the end I left it as it was.

How evil are those approaches or the relying on fields name (the field 
can put something other or even more values to record in its save 
mehtod.) in command construction? Do we want to keep those constrains 
(field should save under its name only one value)?


-- 
Petr Vobornik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0067-Added-IP-address-validator-to-Host-and-DNS-record-ad.patch
Type: text/x-patch
Size: 8634 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120109/2a894f76/attachment.bin>