[#4443] Changed to reset N

diff --git a/changelog_unreleased/4443-fqdn-option-flags-lenient-parsing b/changelog_unreleased/4443-fqdn-option-flags-lenient-parsing
index 84d4876fdd9ec30c4eb297c22f1e680379e73b64..a72328b37244ebdf28e08eabbaf0037b981ef213 100644 (file)
--- a/changelog_unreleased/4443-fqdn-option-flags-lenient-parsing
+++ b/changelog_unreleased/4443-fqdn-option-flags-lenient-parsing
@@ -1,4 +1,4 @@
 [func]         fdupont
        Extended lenient parsing of v4 "fqdn" and v6 "client-fqdn"
-       options to skip options with bad flags.
+       options to fix options with bad flags.
        (Gitlab #4443)

diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst
index bb3bf0bff54ab78507618ae13763e419ef24796e..3871daebb6e3765837e5c24c5c8668d73a049618 100644 (file)
--- a/doc/sphinx/arm/dhcp4-srv.rst
+++ b/doc/sphinx/arm/dhcp4-srv.rst
@@ -8872,7 +8872,8 @@ or in terms of the log message above, the tuple length ``y`` becomes ``x``.
 
 Starting with Kea version 2.5.8, this parsing is extended to silently ignore
 FQDN (81) options with some invalid domain names, and starting with Kea
-version 3.1.9 with invalid flags (i.e. 'S" and 'N' flags set to 1).
+version 3.1.9 to fix invalid flags, i.e. when 'S' and 'N' flags set to 1
+the 'N' flag is reset to 0 for compatibility with ISC DHCP behavior.
 
 Ignore DHCP Server Identifier
 -----------------------------

diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst
index a85305ff974926ba121e0caf87cda9f6f6cb7529..8db27326447c63077cdeb94c28258f7cb82d18f4 100644 (file)
--- a/doc/sphinx/arm/dhcp6-srv.rst
+++ b/doc/sphinx/arm/dhcp6-srv.rst
@@ -8740,7 +8740,8 @@ MiNID.
 
 Starting with Kea version 2.5.8, this parsing is extended to silently ignore
 client-fqdn (39) options with some invalid domain names, and starting with Kea
-version        3.1.9 with invalid flags (i.e. 'S" and 'N' flags set to 1).
+version 3.1.9 to fix invalid flags, i.e. when 'S' and 'N' flags set to 1
+the 'N' flag is reset to 0 for compatibility with ISC DHCP behavior.
 
 .. _dhcp6_allocation_strategies:
 

diff --git a/src/lib/dhcp/option4_client_fqdn.cc b/src/lib/dhcp/option4_client_fqdn.cc
index 0da2503c20af791b45a5e2d3b6deaffa5ba4b087..9826a80c910aa8718a758eab2890712e7b73c7b5 100644 (file)
--- a/src/lib/dhcp/option4_client_fqdn.cc
+++ b/src/lib/dhcp/option4_client_fqdn.cc
@@ -153,7 +153,7 @@ Option4ClientFqdnImpl::Option4ClientFqdnImpl(OptionBufferConstIter first,
         checkFlags(flags_, false);
     } catch (const InvalidOption4FqdnFlags& ex) {
         if (Option::lenient_parsing_) {
-            isc_throw(SkipThisOptionError, ex.what());
+            flags_ &= ~Option4ClientFqdn::FLAG_N;
         } else {
             throw;
         }
@@ -521,7 +521,7 @@ Option4ClientFqdn::unpack(OptionBufferConstIter first,
         impl_->checkFlags(impl_->flags_, false);
     } catch (const InvalidOption4FqdnFlags& ex) {
         if (Option::lenient_parsing_) {
-            isc_throw(SkipThisOptionError, ex.what());
+            impl_->flags_ &= ~Option4ClientFqdn::FLAG_N;
         } else {
             throw;
         }

diff --git a/src/lib/dhcp/option6_client_fqdn.cc b/src/lib/dhcp/option6_client_fqdn.cc
index 2f1e71c37306ae2b6354817a7ae3810550e11d5a..3cd589d7fb482cfe2c935c67d7f98e8ad13778ec 100644 (file)
--- a/src/lib/dhcp/option6_client_fqdn.cc
+++ b/src/lib/dhcp/option6_client_fqdn.cc
@@ -126,7 +126,7 @@ Option6ClientFqdnImpl::Option6ClientFqdnImpl(OptionBufferConstIter first,
         checkFlags(flags_, false);
     } catch (const InvalidOption6FqdnFlags& ex) {
         if (Option::lenient_parsing_) {
-            isc_throw(SkipThisOptionError, ex.what());
+            flags_ &= ~Option6ClientFqdn::FLAG_N;
         } else {
             throw;
         }
@@ -441,7 +441,7 @@ Option6ClientFqdn::unpack(OptionBufferConstIter first,
         impl_->checkFlags(impl_->flags_, false);
     } catch (const InvalidOption6FqdnFlags& ex) {
         if (Option::lenient_parsing_) {
-            isc_throw(SkipThisOptionError, ex.what());
+            impl_->flags_ &= ~Option6ClientFqdn::FLAG_N;
         } else {
             throw;
         }

diff --git a/src/lib/dhcp/tests/option4_client_fqdn_unittest.cc b/src/lib/dhcp/tests/option4_client_fqdn_unittest.cc
index 5b9ce41dbe2d57b7e344345d4a68e17103d34ea1..549fe8cc3923f3e732222df98da860c446382e1d 100644 (file)
--- a/src/lib/dhcp/tests/option4_client_fqdn_unittest.cc
+++ b/src/lib/dhcp/tests/option4_client_fqdn_unittest.cc
@@ -462,9 +462,13 @@ TEST(Option4ClientFqdnTest, constructFromWireInvalidFlags) {
     LenientOptionParsing lop(false);
     EXPECT_THROW(Option4ClientFqdn(in_buf.begin(), in_buf.end()),
                  InvalidOption4FqdnFlags);
+    // Lenient parsing allows this but reset the N bit.
     Option::lenient_parsing_ = true;
-    EXPECT_THROW(Option4ClientFqdn(in_buf.begin(), in_buf.end()),
-                 SkipThisOptionError);
+    boost::scoped_ptr<Option4ClientFqdn> option;
+    EXPECT_NO_THROW(option.reset(new Option4ClientFqdn(in_buf.begin(), in_buf.end())));
+    ASSERT_TRUE(option);
+    EXPECT_TRUE(option->getFlag(Option4ClientFqdn::FLAG_S));
+    EXPECT_FALSE(option->getFlag(Option4ClientFqdn::FLAG_N));
 }
 
 // This test verifies that if invalid domain name is used the constructor
@@ -844,9 +848,11 @@ TEST(Option4ClientFqdnTest, unpack) {
     LenientOptionParsing lop(false);
     EXPECT_THROW(option->unpack(in_buf.begin(), in_buf.end()),
                  InvalidOption4FqdnFlags);
+    // Lenient parsing allows bad flags but reset the N bit.
     Option::lenient_parsing_ = true;
-    EXPECT_THROW(option->unpack(in_buf.begin(), in_buf.end()),
-                 SkipThisOptionError);
+    ASSERT_NO_THROW(option->unpack(in_buf.begin(), in_buf.end()));
+    EXPECT_TRUE(option->getFlag(Option4ClientFqdn::FLAG_S));
+    EXPECT_FALSE(option->getFlag(Option4ClientFqdn::FLAG_N));
 }
 
 // This test verifies that on-wire option data holding partial domain name

diff --git a/src/lib/dhcp/tests/option6_client_fqdn_unittest.cc b/src/lib/dhcp/tests/option6_client_fqdn_unittest.cc
index e23893d18bfcec3fd32ee63b2fccf007e5e76e49..81dbcc5502d289ac6b407eb43616047a4f310d9e 100644 (file)
--- a/src/lib/dhcp/tests/option6_client_fqdn_unittest.cc
+++ b/src/lib/dhcp/tests/option6_client_fqdn_unittest.cc
@@ -413,9 +413,13 @@ TEST(Option6ClientFqdnTest, constructFromWireInvalidFlags) {
     LenientOptionParsing lop(false);
     EXPECT_THROW(Option6ClientFqdn(in_buf.begin(), in_buf.end()),
                  InvalidOption6FqdnFlags);
+    // Lenient parsing allows this but reset the N bit.
     Option::lenient_parsing_ = true;
-    EXPECT_THROW(Option6ClientFqdn(in_buf.begin(), in_buf.end()),
-                 SkipThisOptionError);
+    boost::scoped_ptr<Option6ClientFqdn> option;
+    EXPECT_NO_THROW(option.reset(new Option6ClientFqdn(in_buf.begin(), in_buf.end())));
+    ASSERT_TRUE(option);
+    EXPECT_TRUE(option->getFlag(Option6ClientFqdn::FLAG_S));
+    EXPECT_FALSE(option->getFlag(Option6ClientFqdn::FLAG_N));
 }
 
 // This test verifies that if invalid domain name is used the constructor
@@ -712,9 +716,11 @@ TEST(Option6ClientFqdnTest, unpack) {
     LenientOptionParsing lop(false);
     EXPECT_THROW(option->unpack(in_buf.begin(), in_buf.end()),
                  InvalidOption6FqdnFlags);
+    // Lenient parsing allows bad flags but reset the N bit.
     Option::lenient_parsing_ = true;
-    EXPECT_THROW(option->unpack(in_buf.begin(), in_buf.end()),
-                 SkipThisOptionError);
+    ASSERT_NO_THROW(option->unpack(in_buf.begin(), in_buf.end()));
+    EXPECT_TRUE(option->getFlag(Option6ClientFqdn::FLAG_S));
+    EXPECT_FALSE(option->getFlag(Option6ClientFqdn::FLAG_N));
 }
 
 // This test verifies that on-wire option data holding partial domain name