Skip to content

Commit 60e75e5

Browse files
ibauersachsibauersachs
committed
Fix section size in header after message normalization
Fixes: 2073a0c Closes #384
1 parent 5609998 commit 60e75e5

4 files changed

Lines changed: 59 additions & 18 deletions

File tree

‎src/main/java/org/xbill/DNS/Message.java‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -810,9 +810,11 @@ public Message normalize(Message query, boolean throwOnIrrelevantRecord)
810810
List<RRset> additionalSectionSets = getSectionRRsets(Section.ADDITIONAL);
811811
List<RRset> authoritySectionSets = getSectionRRsets(Section.AUTHORITY);
812812

813-
List<RRset> cleanedAnswerSection = new ArrayList<>();
814-
List<RRset> cleanedAuthoritySection = new ArrayList<>();
815-
List<RRset> cleanedAdditionalSection = new ArrayList<>();
813+
@SuppressWarnings("unchecked")
814+
List<RRset>[] cleanedSection = new ArrayList[4];
815+
cleanedSection[Section.ANSWER] = new ArrayList<>();
816+
cleanedSection[Section.AUTHORITY] = new ArrayList<>();
817+
cleanedSection[Section.ADDITIONAL] = new ArrayList<>();
816818
boolean hadNsInAuthority = false;
817819

818820
// For the ANSWER section, remove all "irrelevant" records and add synthesized CNAMEs from
@@ -843,7 +845,7 @@ public Message normalize(Message query, boolean throwOnIrrelevantRecord)
843845
// If DNAME was queried, don't attempt to synthesize CNAME
844846
if (query.getQuestion().getType() != Type.DNAME) {
845847
// The DNAME is valid, accept it
846-
cleanedAnswerSection.add(rrset);
848+
cleanedSection[Section.ANSWER].add(rrset);
847849

848850
// Check if the next rrset is correct CNAME, otherwise synthesize a CNAME
849851
RRset nextRRSet = answerSectionSets.size() >= i + 2 ? answerSectionSets.get(i + 1) : null;
@@ -863,7 +865,7 @@ public Message normalize(Message query, boolean throwOnIrrelevantRecord)
863865

864866
// Add a synthesized CNAME; TTL=0 to avoid caching
865867
Name dnameTarget = sname.fromDNAME(dname);
866-
cleanedAnswerSection.add(
868+
cleanedSection[Section.ANSWER].add(
867869
new RRset(new CNAMERecord(sname, dname.getDClass(), 0, dnameTarget)));
868870
sname = dnameTarget;
869871

@@ -872,7 +874,7 @@ public Message normalize(Message query, boolean throwOnIrrelevantRecord)
872874
for (i++; i < answerSectionSets.size(); i++) {
873875
rrset = answerSectionSets.get(i);
874876
if (rrset.getName().equals(oldSname)) {
875-
cleanedAnswerSection.add(rrset);
877+
cleanedSection[Section.ANSWER].add(rrset);
876878
} else {
877879
break;
878880
}
@@ -943,14 +945,14 @@ public Message normalize(Message query, boolean throwOnIrrelevantRecord)
943945
}
944946

945947
sname = ((CNAMERecord) rrset.first()).getTarget();
946-
cleanedAnswerSection.add(rrset);
948+
cleanedSection[Section.ANSWER].add(rrset);
947949

948950
// In CNAME ANY response, can have data after CNAME
949951
if (query.getQuestion().getType() == Type.ANY) {
950952
for (i++; i < answerSectionSets.size(); i++) {
951953
rrset = answerSectionSets.get(i);
952954
if (rrset.getName().equals(oldSname)) {
953-
cleanedAnswerSection.add(rrset);
955+
cleanedSection[Section.ANSWER].add(rrset);
954956
} else {
955957
break;
956958
}
@@ -973,9 +975,9 @@ public Message normalize(Message query, boolean throwOnIrrelevantRecord)
973975
}
974976

975977
// Mark the additional names from relevant RRset as OK
976-
cleanedAnswerSection.add(rrset);
978+
cleanedSection[Section.ANSWER].add(rrset);
977979
if (sname.equals(rrset.getName())) {
978-
addAdditionalRRset(rrset, additionalSectionSets, cleanedAdditionalSection);
980+
addAdditionalRRset(rrset, additionalSectionSets, cleanedSection[Section.ADDITIONAL]);
979981
}
980982
}
981983

@@ -1045,15 +1047,25 @@ public Message normalize(Message query, boolean throwOnIrrelevantRecord)
10451047
}
10461048
}
10471049

1048-
cleanedAuthoritySection.add(rrset);
1049-
addAdditionalRRset(rrset, additionalSectionSets, cleanedAdditionalSection);
1050+
cleanedSection[Section.AUTHORITY].add(rrset);
1051+
addAdditionalRRset(rrset, additionalSectionSets, cleanedSection[Section.ADDITIONAL]);
10501052
}
10511053

10521054
Message cleanedMessage = new Message(this.getHeader());
10531055
cleanedMessage.sections[Section.QUESTION] = this.sections[Section.QUESTION];
1054-
cleanedMessage.sections[Section.ANSWER] = rrsetListToRecords(cleanedAnswerSection);
1055-
cleanedMessage.sections[Section.AUTHORITY] = rrsetListToRecords(cleanedAuthoritySection);
1056-
cleanedMessage.sections[Section.ADDITIONAL] = rrsetListToRecords(cleanedAdditionalSection);
1056+
for (int section : new int[] {Section.ANSWER, Section.AUTHORITY, Section.ADDITIONAL}) {
1057+
cleanedMessage.sections[section] = rrsetListToRecords(cleanedSection[section]);
1058+
1059+
// Fixup counts in the header
1060+
cleanedMessage
1061+
.getHeader()
1062+
.setCount(
1063+
section,
1064+
cleanedMessage.sections[section] == null
1065+
? 0
1066+
: cleanedMessage.sections[section].size());
1067+
}
1068+
10571069
return cleanedMessage;
10581070
}
10591071

‎src/test/java/org/xbill/DNS/MessageTest.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
//
3636
package org.xbill.DNS;
3737

38+
import static org.assertj.core.api.Assertions.assertThat;
3839
import static org.junit.jupiter.api.Assertions.assertEquals;
3940
import static org.junit.jupiter.api.Assertions.assertThrows;
4041
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -177,7 +178,9 @@ void normalize() throws WireParseException {
177178
response.addRecord(queryRecord, Section.QUESTION);
178179
response.addRecord(queryRecord, Section.ADDITIONAL);
179180
response = response.normalize(query, true);
180-
assertTrue(response.getSection(Section.ANSWER).isEmpty());
181-
assertTrue(response.getSection(Section.ADDITIONAL).isEmpty());
181+
assertThat(response.getSection(Section.ANSWER)).isEmpty();
182+
assertThat(response.getHeader().getCount(Section.ANSWER)).isZero();
183+
assertThat(response.getSection(Section.ADDITIONAL)).isEmpty();
184+
assertThat(response.getHeader().getCount(Section.ADDITIONAL)).isZero();
182185
}
183186
}

‎src/test/java/org/xbill/DNS/TSIGTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package org.xbill.DNS;
33

44
import static org.assertj.core.api.Assertions.assertThat;
5+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
56
import static org.junit.jupiter.api.Assertions.assertEquals;
67
import static org.junit.jupiter.api.Assertions.assertFalse;
78
import static org.junit.jupiter.api.Assertions.assertNotEquals;
@@ -520,6 +521,19 @@ TCPClient createTcpClient(Duration timeout) throws IOException {
520521
assertEquals(202, handler.getRecords().size());
521522
}
522523

524+
@Test
525+
void invalidAdditionalCount() {
526+
Message q = Message.newQuery(Record.newRecord(Name.root, Type.A, DClass.IN));
527+
Message m = new Message();
528+
m.addRecord(Record.newRecord(Name.root, Type.A, DClass.IN), Section.QUESTION);
529+
m.addRecord(Record.newRecord(Name.root, Type.A, DClass.IN), Section.ANSWER);
530+
m.addRecord(
531+
Record.newRecord(Name.fromConstantString("example.com."), Type.A, DClass.IN),
532+
Section.ADDITIONAL);
533+
assertDoesNotThrow(m::getTSIG);
534+
assertDoesNotThrow(() -> m.normalize(q).getTSIG());
535+
}
536+
523537
@Getter
524538
private static class ZoneBuilderAxfrHandler implements ZoneTransferIn.ZoneTransferHandler {
525539
private final List<Record> records = new ArrayList<>();

‎src/test/java/org/xbill/DNS/dnssec/TestBase.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: BSD-3-Clause
22
package org.xbill.DNS.dnssec;
33

4+
import static org.assertj.core.api.Assertions.assertThat;
45
import static org.junit.jupiter.api.Assertions.assertEquals;
56
import static org.junit.jupiter.api.Assertions.fail;
67
import static org.mockito.Mockito.mock;
@@ -126,7 +127,18 @@ private void starting(TestInfo description) {
126127

127128
Message m;
128129
while ((m = messageReader.readMessage(r)) != null) {
130+
for (int i = 0; i < 4; i++) {
131+
assertThat(m.getHeader().getCount(i))
132+
.withFailMessage("Before normalization")
133+
.isEqualTo(m.getSection(i).size());
134+
}
135+
129136
m = m.normalize(Message.newQuery(m.getQuestion()), true);
137+
for (int i = 0; i < 4; i++) {
138+
assertThat(m.getHeader().getCount(i))
139+
.withFailMessage("After normalization")
140+
.isEqualTo(m.getSection(i).size());
141+
}
130142
queryResponsePairs.put(key(m), m);
131143
}
132144

@@ -286,7 +298,7 @@ protected String getEdeText(Message m) {
286298
.flatMap(
287299
opt ->
288300
opt.getOptions(Code.EDNS_EXTENDED_ERROR).stream()
289-
.filter(o -> o instanceof ExtendedErrorCodeOption)
301+
.filter(ExtendedErrorCodeOption.class::isInstance)
290302
.findFirst()
291303
.map(o -> ((ExtendedErrorCodeOption) o).getText()))
292304
.orElse(null);

0 commit comments

Comments
 (0)