RecordItems iterator stops on mismatching record type #1

Closed
opened 2018-11-12 04:31:39 +13:00 by stbuehler · 5 comments
stbuehler commented 2018-11-12 04:31:39 +13:00 (Migrated from github.com)

All sections can carry records of different type; the iterator should skip all records with different type.

Right now it returns None (through the .ok() call at the end), which is the "universal" stop signal for iterators.

The most important case for this is when iterating over the answer section which contains a CNAME (usually as the first record in the section).

The iterator should also either returns the parse errors (after skipping the records with different type) or skip broken records; returning None is again not a good option.

All sections can carry records of different type; the iterator should skip all records with different type. Right now it returns `None` (through the `.ok()` call at the end), which is the "universal" stop signal for iterators. The most important case for this is when iterating over the answer section which contains a CNAME (usually as the first record in the section). The iterator should also either returns the parse errors (after skipping the records with different type) or skip broken records; returning `None` is again not a good option.
mikedilger commented 2018-11-12 12:21:33 +13:00 (Migrated from github.com)

Thanks for finding this. By way of explanation, I coded this library for a project that a client cancelled, so it may not have much real world exposure yet, hence bugs like this.

Can you check if the code on the 'fix1' branch fixes this for you? I don't easily have access to the situation you describe. Thanks.

Thanks for finding this. By way of explanation, I coded this library for a project that a client cancelled, so it may not have much real world exposure yet, hence bugs like this. Can you check if the code on the 'fix1' branch fixes this for you? I don't easily have access to the situation you describe. Thanks.
stbuehler commented 2018-11-12 19:37:34 +13:00 (Migrated from github.com)

I'll try to test it later, but it looks good so far regarding my original issue. Small nit: if the lower level parse error should lead to aborting the sequence you might want to set index to len before returning None.

I'll try to test it later, but it looks good so far regarding my original issue. Small nit: if the lower level parse error should lead to aborting the sequence you might want to set index to len before returning `None`.
stbuehler commented 2018-11-15 11:49:18 +13:00 (Migrated from github.com)

I can confirm it fixes the issue. If you want to try yourself, query e.g. for mail.google.com - for me this returns this in the answer section (i.e. mixed types):

mail.google.com.	430062	IN	CNAME	googlemail.l.google.com.
googlemail.l.google.com. 128	IN	A	172.217.22.37
I can confirm it fixes the issue. If you want to try yourself, query e.g. for `mail.google.com` - for me this returns this in the answer section (i.e. mixed types): ``` mail.google.com. 430062 IN CNAME googlemail.l.google.com. googlemail.l.google.com. 128 IN A 172.217.22.37 ```
mikedilger commented 2018-11-15 16:56:17 +13:00 (Migrated from github.com)

I just updated to 0.1.4 and published.

I just updated to 0.1.4 and published.
stbuehler commented 2018-11-17 23:39:44 +13:00 (Migrated from github.com)

Thanks for fixing, and thanks for providing the crate in the first place.

I decided to roll my own iterator anyway (because I really wanted "proper" error handling), and build an AddressRecord trait on top of RecordData. See https://github.com/stbuehler/rust-alookup/blob/master/src/resolv_ext.rs if you're interested.

Thanks for fixing, and thanks for providing the crate in the first place. I decided to roll my own iterator anyway (because I really wanted "proper" error handling), and build an `AddressRecord` trait on top of `RecordData`. See https://github.com/stbuehler/rust-alookup/blob/master/src/resolv_ext.rs if you're interested.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
mikedilger/resolv-rs#1
No description provided.