Bug 1248770 - change sdp_unittests to cope with diverse c++ standard interpretations; r=jesup
authorNathan Froyd <froydnj@mozilla.com>
Tue, 16 Feb 2016 21:09:34 -0500
changeset 331900 edd76ca3717f581c9c827396160b4e807fbcb781
parent 331899 38f5491bab4cc81b6e8b6f1a7b1164f60fb9fb13
child 331901 500bdcc53e5419f1b8303dc96bd7c790c0a3ad4b
push id11113
push userrjesup@wgate.com
push dateThu, 18 Feb 2016 19:00:12 +0000
reviewersjesup
bugs1248770
milestone47.0a1
Bug 1248770 - change sdp_unittests to cope with diverse c++ standard interpretations; r=jesup The C++ standard, [facet.num.get.virtuals], defines the method to be used for reading numeric values from an iterator. The core loop is defined thusly in N3242 (the draft for the C++11 standard): Stage 2: If in==end then stage 2 terminates. Otherwise a charT is taken from in and local variables are initialized as if by char_type ct = *in; char c = src[find(atoms, atoms + sizeof(src) - 1, ct) - atoms]; if (ct == use_facet<numpunct<charT> >(loc).decimal_point()) c = '.'; bool discard = ct == use_facet<numpunct<charT> >(loc).thousands_sep() && use_facet<numpunct<charT> >(loc).grouping().length() != 0; where the values src and atoms are defined as if by: static const char src[] = "0123456789abcdefxABCDEFX+-"; char_type atoms[sizeof(src)]; use_facet<ctype<charT> >(loc).widen(src, src + sizeof(src), atoms); for this value of loc. If discard is true, then if '.' has not yet been accumulated, then the position of the character is remembered, but the character is otherwise ignored. Otherwise, if '.' has already been accumulated, the character is discarded and Stage 2 terminates. If the character is either discarded or accumulated then in is advanced by ++in and processing returns to the beginning of stage 2. Stage 3: The sequence of chars accumulated in stage 2 (the field) is converted to a numeric value by the rules of one of the functions declared in the header <cstdlib>: - For a signed integer value, the function strtoll. - For an unsigned integer value, the function strtoull. - For a floating-point value, the function strtold. The important part for our purposes here is the line: char c = src[find(atoms, atoms + sizeof(src) - 1, ct) - atoms]; which implies that we are to accumulate any and all characters that might be numerical constituents. According to the spec text, we might accumulate a long run of numeric constituents, only to decide in stage 3 that our accumulated string cannot be a valid number. Please note that this includes characters like 'x' and 'X' which are only valid as part of a hexadecimal prefix. sdp_unittests has a number of tests that look like: ParseInvalid<SdpImageattrAttributeList::XYRange>("[x", 1); The test converts the input string to a stringstream, and attempts to read an integer from the stream starting after the '[' character. The test expects that no input from the string stream will be consumed, as the character 'x' cannot begin any number, and thus the position of the stream after failure will be 1. This behavior is consistent with MSVC's standard library, libstdc++, and stlport. However, libc++ uses a different algorithm that appears to hew more closely to the letter of the spec, and consumes the 'x' character as being a valid constituent character ("accumulates" in the above text). The string is rejected as an invalid integer, yet the position of the string stream afterwards is different from what the test expects, and we therefore fail. This patch therefore alters a number of tests to use a different invalid character, 'v', that both the incremental algorithm (MSVC, libstdc++, stlport) and the all-at-once algorithm (libc++) will recognize as not being a valid constituent character and stop the parsing early, as expected. You might think that specifying the base for numeric input as std::dec would work, and it partially does, but reading floating-point numbers still reads the 'x' characters (!).
media/webrtc/signaling/test/sdp_unittests.cpp
--- a/media/webrtc/signaling/test/sdp_unittests.cpp
+++ b/media/webrtc/signaling/test/sdp_unittests.cpp
@@ -2820,50 +2820,50 @@ ParseInvalid(const std::string& input, s
   // For a human to eyeball to make sure the error strings look sane
   std::cout << "\"" << input << "\" - " << error << std::endl; \
 }
 
 TEST(NewSdpTestNoFixture, CheckImageattrXYRangeParseInvalid)
 {
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[-1", 1);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[-", 1);
-  ParseInvalid<SdpImageattrAttributeList::XYRange>("[-x", 1);
+  ParseInvalid<SdpImageattrAttributeList::XYRange>("[-v", 1);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:-1", 5);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:16:-1", 8);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640,-1", 5);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640,-]", 5);
-  ParseInvalid<SdpImageattrAttributeList::XYRange>("-x", 0);
+  ParseInvalid<SdpImageattrAttributeList::XYRange>("-v", 0);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("-1", 0);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("", 0);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[", 1);
-  ParseInvalid<SdpImageattrAttributeList::XYRange>("[x", 1);
+  ParseInvalid<SdpImageattrAttributeList::XYRange>("[v", 1);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[", 1);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[ 640", 1);
   // It looks like the overflow detection only happens once the whole number
   // is scanned...
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[99999999999999999:", 18);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640", 4);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:", 5);
-  ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:x", 5);
+  ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:v", 5);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:16", 7);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:16:", 8);
-  ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:16:x", 8);
+  ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:16:v", 8);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:16:320]", 11);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:16:320", 11);
-  ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:16:320x", 11);
+  ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:16:320v", 11);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:1024", 9);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:320]", 8);
-  ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:1024x", 9);
+  ParseInvalid<SdpImageattrAttributeList::XYRange>("[640:1024v", 9);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640,", 5);
-  ParseInvalid<SdpImageattrAttributeList::XYRange>("[640,x", 5);
+  ParseInvalid<SdpImageattrAttributeList::XYRange>("[640,v", 5);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640]", 4);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640x", 4);
   ParseInvalid<SdpImageattrAttributeList::XYRange>("[640,]", 5);
   ParseInvalid<SdpImageattrAttributeList::XYRange>(" ", 0);
-  ParseInvalid<SdpImageattrAttributeList::XYRange>("x", 0);
+  ParseInvalid<SdpImageattrAttributeList::XYRange>("v", 0);
 }
 
 static SdpImageattrAttributeList::SRange
 ParseSRange(const std::string& input)
 {
   std::istringstream is(input + ",");
   std::string error;
   SdpImageattrAttributeList::SRange range;
@@ -2903,41 +2903,41 @@ TEST(NewSdpTestNoFixture, CheckImageattr
     ASSERT_FLOAT_EQ(0.2f, range.max);
   }
 }
 
 TEST(NewSdpTestNoFixture, CheckImageattrSRangeParseInvalid)
 {
   ParseInvalid<SdpImageattrAttributeList::SRange>("", 0);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[", 1);
-  ParseInvalid<SdpImageattrAttributeList::SRange>("[x", 1);
+  ParseInvalid<SdpImageattrAttributeList::SRange>("[v", 1);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[-1", 1);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[", 1);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[-", 1);
-  ParseInvalid<SdpImageattrAttributeList::SRange>("[x", 1);
+  ParseInvalid<SdpImageattrAttributeList::SRange>("[v", 1);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[ 0.2", 1);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[10.1-", 5);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[0.08-", 5);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2", 4);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2-", 5);
-  ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2-x", 5);
+  ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2-v", 5);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2--1", 5);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2-0.3", 8);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2-0.1]", 8);
-  ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2-0.3x", 8);
+  ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2-0.3v", 8);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2,", 5);
-  ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2,x", 5);
+  ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2,v", 5);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2,-1", 5);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2]", 4);
-  ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2x", 4);
+  ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2v", 4);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2,]", 5);
   ParseInvalid<SdpImageattrAttributeList::SRange>("[0.2,-]", 5);
   ParseInvalid<SdpImageattrAttributeList::SRange>(" ", 0);
-  ParseInvalid<SdpImageattrAttributeList::SRange>("x", 0);
-  ParseInvalid<SdpImageattrAttributeList::SRange>("-x", 0);
+  ParseInvalid<SdpImageattrAttributeList::SRange>("v", 0);
+  ParseInvalid<SdpImageattrAttributeList::SRange>("-v", 0);
   ParseInvalid<SdpImageattrAttributeList::SRange>("-1", 0);
 }
 
 static SdpImageattrAttributeList::PRange
 ParsePRange(const std::string& input)
 {
   std::istringstream is(input + ",");
   std::string error;
@@ -2954,37 +2954,37 @@ TEST(NewSdpTestNoFixture, CheckImageattr
   ASSERT_FLOAT_EQ(0.1f, range.min);
   ASSERT_FLOAT_EQ(9.9999f, range.max);
 }
 
 TEST(NewSdpTestNoFixture, CheckImageattrPRangeParseInvalid)
 {
   ParseInvalid<SdpImageattrAttributeList::PRange>("", 0);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[", 1);
-  ParseInvalid<SdpImageattrAttributeList::PRange>("[x", 1);
+  ParseInvalid<SdpImageattrAttributeList::PRange>("[v", 1);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[-1", 1);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[", 1);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[-", 1);
-  ParseInvalid<SdpImageattrAttributeList::PRange>("[x", 1);
+  ParseInvalid<SdpImageattrAttributeList::PRange>("[v", 1);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[ 0.2", 1);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[10.1-", 5);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[0.08-", 5);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2", 4);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2-", 5);
-  ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2-x", 5);
+  ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2-v", 5);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2--1", 5);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2-0.3", 8);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2-0.1]", 8);
-  ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2-0.3x", 8);
+  ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2-0.3v", 8);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2,", 4);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2:", 4);
   ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2]", 4);
-  ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2x", 4);
+  ParseInvalid<SdpImageattrAttributeList::PRange>("[0.2v", 4);
   ParseInvalid<SdpImageattrAttributeList::PRange>(" ", 0);
-  ParseInvalid<SdpImageattrAttributeList::PRange>("x", 0);
+  ParseInvalid<SdpImageattrAttributeList::PRange>("v", 0);
   ParseInvalid<SdpImageattrAttributeList::PRange>("-x", 0);
   ParseInvalid<SdpImageattrAttributeList::PRange>("-1", 0);
 }
 
 static SdpImageattrAttributeList::Set
 ParseSet(const std::string& input)
 {
   std::istringstream is(input + " ");
@@ -3126,35 +3126,35 @@ TEST(NewSdpTestNoFixture, CheckImageattr
   ParseInvalid<SdpImageattrAttributeList::Set>("", 0);
   ParseInvalid<SdpImageattrAttributeList::Set>("x", 0);
   ParseInvalid<SdpImageattrAttributeList::Set>("[", 1);
   ParseInvalid<SdpImageattrAttributeList::Set>("[=", 2);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x", 2);
   ParseInvalid<SdpImageattrAttributeList::Set>("[y=", 3);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=[", 4);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320", 6);
-  ParseInvalid<SdpImageattrAttributeList::Set>("[x=320x", 6);
+  ParseInvalid<SdpImageattrAttributeList::Set>("[x=320v", 6);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,", 7);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,=", 8);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,x", 8);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,x=", 9);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=[", 10);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240", 12);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240x", 12);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,", 13);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,q=", 15);
-  ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,q=x", 15);
+  ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,q=v", 15);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,q=0.5", 18);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,q=0.5,", 19);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,q=0.5,]", 20);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,q=0.5,=]", 20);
-  ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,q=0.5,sar=x]", 23);
+  ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,q=0.5,sar=v]", 23);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,q=0.5,q=0.4", 21);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,sar=", 17);
-  ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,sar=x", 17);
+  ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,sar=v", 17);
   ParseInvalid<SdpImageattrAttributeList::Set>(
       "[x=320,y=240,sar=[0.5-0.6],sar=[0.7-0.8]", 31);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,par=", 17);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,par=x", 17);
   ParseInvalid<SdpImageattrAttributeList::Set>(
       "[x=320,y=240,par=[0.5-0.6],par=[0.7-0.8]", 31);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,foo=", 17);
   ParseInvalid<SdpImageattrAttributeList::Set>("[x=320,y=240,foo=x", 18);