Stricter syntax checking for JSBSim.

JSBSim was not checking the tag names inside <test> and <condition> statements and this has led to abusive use of tags unsupported by JSBSim such as <and> and <or>. The problem is that JSBSim was not interpreting the <or> statement as the user meant it: JSBSim interpreted it as <test logic="AND"> while the user obviously meant <test logic="OR">. As a result JSBSim silently swallowed the tag and gave results which were not was the user expected without any obvious indication about what was going wrong.

As a result, this commit implements stricter syntax checking and rejects illegal tags within <test> and <condition> statements.

This commit also introduces stricter checking on XML elements which allow either a real value or a property name to be used. JSBSim now rejects the statements where the property name is specified inside a <property> statement. For example, the follwoing statement is rejected:

<clipto>
  <min> <property> /fdm/jsbsim/some/property </property> </min>
...

The correct syntax being
<clipto>
  <min> /fdm/jsbsim/some/property </min>

src/FDM/JSBSim/input_output/FGPropertyReader.cpp

@@ -53,11 +53,10 @@ CLASS IMPLEMENTATION
 
 bool FGPropertyReader::ResetToIC(void)
 {
-  map<SGPropertyNode_ptr, double>::iterator it = interface_prop_initial_value.begin();
+  for (auto v: interface_prop_initial_value) {
-  for (;it != interface_prop_initial_value.end(); ++it) {
+    SGPropertyNode* node = v.first;
-    SGPropertyNode* node = it->first;
     if (!node->getAttribute(SGPropertyNode::PRESERVE))
-      node->setDoubleValue(it->second);
+      node->setDoubleValue(v.second);
   }
 
   return true;
@@ -67,9 +66,6 @@ bool FGPropertyReader::ResetToIC(void)
 
 void FGPropertyReader::Load(Element* el, FGPropertyManager* PM, bool override)
 {
-  // Interface properties are all stored in the interface properties array.
-  string interface_property_string = "";
-
   Element *property_element = el->FindElement("property");
   if (property_element && FGJSBBase::debug_lvl > 0) {
     cout << endl << "    ";
@@ -81,12 +77,12 @@ void FGPropertyReader::Load(Element* el, FGPropertyManager* PM, bool override)
   }
 
   while (property_element) {
-    SGPropertyNode* node = 0;
+    SGPropertyNode* node = nullptr;
     double value=0.0;
     if ( ! property_element->GetAttributeValue("value").empty())
       value = property_element->GetAttributeValueAsNumber("value");
 
-    interface_property_string = property_element->GetDataLine();
+    string interface_property_string = property_element->GetDataLine();
     if (PM->HasNode(interface_property_string)) {
       if (override) {
         node = PM->GetNode(interface_property_string);

src/FDM/JSBSim/math/FGCondition.cpp

@@ -36,6 +36,7 @@ INCLUDES
 
 #include <iostream>
 #include <cstdlib>
+#include <stdexcept>
 
 #include "FGCondition.h"
 #include "FGPropertyValue.h"
@@ -65,20 +66,30 @@ FGCondition::FGCondition(Element* element, FGPropertyManager* PropertyManager)
     else { // error
       cerr << element->ReadFrom()
            << "Unrecognized LOGIC token " << logic << endl;
+      throw std::invalid_argument("Illegal argument");
     }
   } else {
     Logic = eAND; // default
   }
 
-  Element* condition_element = element->GetElement();
-
   for (unsigned int i=0; i<element->GetNumDataLines(); i++) {
     string data = element->GetDataLine(i);
-    conditions.push_back(new FGCondition(data, PropertyManager,
+    conditions.push_back(new FGCondition(data, PropertyManager, element));
-                                         condition_element));
   }
 
+  Element* condition_element = element->GetElement();
+  const string& elName = element->GetName();
+
   while (condition_element) {
+    string tagName = condition_element->GetName();
+
+    if (tagName != elName) {
+      cerr << condition_element->ReadFrom()
+           << "Unrecognized tag <" << tagName << "> in the condition statement."
+           << endl;
+      throw std::invalid_argument("Illegal argument");
+    }
+
     conditions.push_back(new FGCondition(condition_element, PropertyManager));
     condition_element = element->GetNextElement();
   }

src/FDM/JSBSim/math/FGParameterValue.h

@@ -34,8 +34,11 @@
   INCLUDES
   %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%*/
 
+#include <stdexcept>
+
 #include "math/FGRealValue.h"
 #include "math/FGPropertyValue.h"
+#include "input_output/FGXMLElement.h"
 
 /*%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
   FORWARD DECLARATIONS
@@ -60,13 +63,22 @@ class FGPropertyManager;
 class FGParameterValue : public FGParameter
 {
 public:
-  FGParameterValue(const std::string& value, FGPropertyManager* pm) {
+  FGParameterValue(Element* el, FGPropertyManager* pm) {
-    if (is_number(value)) {
+    string value = el->GetDataLine();
-      param = new FGRealValue(atof(value.c_str()));
+
-    } else {
+    if (el->GetNumDataLines() != 1 || value.empty()) {
-      // "value" must be a property if execution passes to here.
+      cerr << el->ReadFrom()
-      param = new FGPropertyValue(value, pm);
+           << "The element <" << el->GetName()
+           << "> must either contain a value number or a property name."
+           << endl;
+      throw invalid_argument("Illegal argument");
     }
+
+    Construct(value, pm);
+  }
+
+  FGParameterValue(const std::string& value, FGPropertyManager* pm) {
+    Construct(value, pm);
   }
 
   double GetValue(void) const override { return param->GetValue(); }
@@ -86,6 +98,15 @@ public:
   }
 private:
   FGParameter_ptr param;
+
+  void Construct(const std::string& value, FGPropertyManager* pm) {
+    if (is_number(value)) {
+      param = new FGRealValue(atof(value.c_str()));
+    } else {
+      // "value" must be a property if execution passes to here.
+      param = new FGPropertyValue(value, pm);
+    }
+  }
 };
 
 typedef SGSharedPtr<FGParameterValue> FGParameterValue_ptr;

src/FDM/JSBSim/models/flight_control/FGDeadBand.cpp

@@ -57,16 +57,14 @@ CLASS IMPLEMENTATION
 FGDeadBand::FGDeadBand(FGFCS* fcs, Element* element)
   : FGFCSComponent(fcs, element)
 {
-
   Width = nullptr;
   gain = 1.0;
 
-  string width_string = "0.0";
   Element* width_element = element->FindElement("width");
   if (width_element)
-    width_string = width_element->GetDataLine();
+    Width = new FGParameterValue(width_element, PropertyManager);
-
+  else
-  Width = new FGParameterValue(width_string, PropertyManager);
+    Width = new FGRealValue(0.0);
 
   if (element->FindElement("gain"))
     gain = element->FindElementValueAsNumber("gain");

src/FDM/JSBSim/models/flight_control/FGFCSComponent.cpp

@@ -168,21 +168,19 @@ FGFCSComponent::FGFCSComponent(FGFCS* _fcs, Element* element) : fcs(_fcs)
       return;
     }
 
-    string clip_string = el->GetDataLine();
+    ClipMin = new FGParameterValue(el, PropertyManager);
-    ClipMin = new FGParameterValue(clip_string, PropertyManager);
 
     el = clip_el->FindElement("max");
     if (!el) {
       cerr << clip_el->ReadFrom()
            << "Element <max> is missing, <clipto> is ignored." << endl;
+      ClipMin = nullptr;
       return;
     }
 
-    clip_string = el->GetDataLine();
+    ClipMax = new FGParameterValue(el, PropertyManager);
-    ClipMax = new FGParameterValue(clip_string, PropertyManager);
 
-    clip_string = clip_el->GetAttributeValue("type");
+    if (clip_el->GetAttributeValue("type") == "cyclic")
-    if (clip_string == "cyclic")
       cyclic_clip = true;
 
     clip = true;

src/FDM/JSBSim/models/flight_control/FGFilter.cpp

@@ -99,8 +99,8 @@ void FGFilter::ReadFilterCoefficients(Element* element, int index)
   coefficient[1] += index;
   
   if ( element->FindElement(coefficient) ) {
-    string property_string = element->FindElementValue(coefficient);
+    C[index] = new FGParameterValue(element->FindElement(coefficient),
-    C[index] = new FGParameterValue(property_string, PropertyManager);
+                                    PropertyManager);
     DynamicFilter |= !C[index]->IsConstant();
   }
 }

src/FDM/JSBSim/models/flight_control/FGGain.cpp

@@ -69,12 +69,11 @@ FGGain::FGGain(FGFCS* fcs, Element* element) : FGFCSComponent(fcs, element)
     }
   }
 
-  string gain_string = "1.0";
   Element* gain_element = element->FindElement("gain");
   if (gain_element)
-    gain_string = gain_element->GetDataLine();
+    Gain = new FGParameterValue(gain_element, PropertyManager);
-
+  else
-  Gain = new FGParameterValue(gain_string, PropertyManager);
+    Gain = new FGRealValue(1.0);
 
   if (Type == "AEROSURFACE_SCALE") {
     Element* scale_element = element->FindElement("domain");

src/FDM/JSBSim/models/flight_control/FGPID.cpp

@@ -64,14 +64,12 @@ FGPID::FGPID(FGFCS* fcs, Element* element) : FGFCSComponent(fcs, element)
 
   if (pid_type == "standard") IsStandard = true;
 
-  string kp_string = "0.0";
   el = element->FindElement("kp");
   if (el)
-    kp_string = el->GetDataLine();
+    Kp = new FGParameterValue(el, PropertyManager);
+  else
+    Kp = new FGRealValue(0.0);
 
-  Kp = new FGParameterValue(kp_string, PropertyManager);
-
-  string ki_string = "0.0";
   el = element->FindElement("ki");
   if (el) {
     string integ_type = el->GetAttributeValue("type");
@@ -87,17 +85,17 @@ FGPID::FGPID(FGFCS* fcs, Element* element) : FGFCSComponent(fcs, element)
       IntType = eAdamsBashforth2;
     }
 
-    ki_string = el->GetDataLine();
+    Ki = new FGParameterValue(el, PropertyManager);
   }
+  else
+    Ki = new FGRealValue(0.0);
 
-  Ki = new FGParameterValue(ki_string, PropertyManager);
 
-  string kd_string = "0.0";
   el = element->FindElement("kd");
   if (el)
-    kd_string = el->GetDataLine();
+    Kd = new FGParameterValue(el, PropertyManager);
-
+  else
-  Kd = new FGParameterValue(kd_string, PropertyManager);
+    Kd = new FGRealValue(0.0);
 
   el = element->FindElement("pvdot");
   if (el)