RDATA lowercase
Some more tests would be handy, will add next week.
Merge request reports
Activity
174 174 */ 175 175 uint32_t knot_rrset_ttl(const knot_rrset_t *rrset); 176 176 177 /*! 178 * \brief Convert one RR into canonical format. 179 * 180 * Owner is always converted to lowercase. RDATA domain names are converted only 181 * for types listed in RFC 4034, Section 6.2, except for NSEC (updated by 182 * RFC 6840, Section 5.1) and A6 (not supported). 183 * 184 * \note If RRSet with more RRs is given to this function, only the first RR 185 * will be converted. Discussed with @jvcelak. Function for converting whole RRSet would have too much overhead - it would have to order the RRSet as well. (If a whole RRSet is not converted to lowercase, it will likely be ordered wrongly.) This is completely useless now, everywhere, where it is used, the server works only with one RR. There is this note in the doxygen, so if in any time we need to convert whole RRSet to canonical format, we may add a new function or modify this one if it proves suitable.
One more reason for leaving it here in this file: the function converts also the owner domain name to lowercase. That is another thing that would have to (unnecessarily) be done on the caller functions (=> redundant code).
Also: the function for getting TTL also takes it only from the first RR (as there is no other way to do it), so I don't think it's a problem that this function works only with one RR. We have no structure for representing one (whole) RR, deal with it.
803 803 while(sp != lstack) { /* consume stack */ 804 804 l = *--sp; /* fetch rightmost label */ 805 805 memcpy(dst, l+1, *l); /* write label */ 806 for (int i = 0; i < *l; ++i) { /* convert to lowercase */ 807 dst[i] = knot_tolower(dst[i]); 808 } This will do.
However, there are some interesting facts about performance of this construct:
- The label will be iterated twice (which will be hopefully optimized by the compiler).
-
knot_tolower
is probably slower thantolower
(except that the second one is known to be broken in some implementations). - De-Asmed algorithm gives a nice performance: https://code.google.com/p/stringencoders/wiki/PerformanceAscii
This will be performed for each lookup (possible multiple times per query). We should make sure, that it has insignificant impact .
522 510 assert(dst && *dst); 523 511 assert(dst_avail); 524 512 assert(dname_cfg); 525 UNUSED(flags); 513 UNUSED(dname_type); 526 514 527 int compr_size = knot_dname_wire_check(*src, *src + *src_avail, dname_cfg->pkt_wire); 515 int compr_size = knot_dname_wire_check(*src, *src + *src_avail, 165 return KNOT_EINVAL; 166 } 167 168 /* Convert owner for all RRSets. */ 169 int ret = knot_dname_to_lower(rrset->owner); 170 if (ret != KNOT_EOK) { 171 return ret; 172 } 173 174 /* Convert DNAMEs in RDATA only for RFC4034 types. */ 175 if (knot_rrtype_should_be_lowercased(rrset->type)) { 176 const rdata_descriptor_t *desc = 177 knot_get_rdata_descriptor(rrset->type); 178 if (desc->type_name == NULL) { 179 desc = knot_get_obsolete_rdata_descriptor(rrset->type); 180 } Yeah, it also came into my mind. Actually, I think the descriptor search might search in the obsolete descriptors automatically. I'm not sure why it is NOT used in some cases. Is there any particular reason?
Now this function is called in only two places, so I think we may keep it that way for a while